Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support hivemind's condenser_api #305

Open
relativityboy opened this issue Jan 2, 2018 · 8 comments
Open

Support hivemind's condenser_api #305

relativityboy opened this issue Jan 2, 2018 · 8 comments
Labels

Comments

@relativityboy
Copy link
Contributor

relativityboy commented Jan 2, 2018

Hive will support apis at multiple endpoints. Specifically the steemd api, both appbase and nonappbase.

Jussi will support routing to these apis via different RPC method prefixes.

We want to support these different prefixes without requiring significant changes to codebases currently using steem-js. We also want to be able to be able to update where these methods point (change the prefixes and appbase/non-appbase structure) without needing to bump-release
steem-js.

This will allow us to keep steem-js stable while we evolve our services.

A sample customApis option.

customApis: [
{
            prefix: 'hive_api.condenser_api', //optional
            useNonAppbCallStructure: true, //optional
            methods: [
                'get_follow_count',
                'get_followers',
                'get_following',
                'get_open_orders',
                'get_dynamic_global_properties',
            ],
}
]
@jnordberg
Copy link
Contributor

jnordberg commented Jan 9, 2018

I'm not a huge fan of this, it makes an already convoluted api more so. I'd prefer if we move to calling all apis with their method names directly, e.g. api.call('get_dynamic_global_properties') and api.call('hive.get_dynamic_global_properties') if you want to speak with hive.

If we want to retire an steemd method and let hive take over we can do that routing in jussi.

@bnchdrff
Copy link
Contributor

bnchdrff commented Jan 9, 2018

is there any downside to having jussi take care of this? it seems like the ideal place to do it, even in the case of running jussi for local dev & setting up weird temporary routing.

@relativityboy
Copy link
Contributor Author

The issues are that

  1. jussi shouldn't care about this stuff We want jussi 1.0 to remain dumb with regard to specific api needs (no api route should need "special" handling ex: request restructuring)
  2. Hive needs to support a 'pure legacy' endpoint
  3. We may need to switch between hive's /legacy (non-appbase style calls), hive / and steemd depending on traffic and performance. All these have different mixes of rpc structure, method prefix, and method name. (this 3rd property is the sometime replacement of the method name with 'call')
  4. This change keeps us from breaking 3rd party code until such time as we can move to our new api client.

I'm definitely in favor of something like api.call('hive.get_dynamic_global_properties') once we eliminate the need to mask switching between baked-in rpc calls like

{
   method: 'someservice.call',
   params: ['blah', 'get_dynamic_global_properties', {realparamsgohere}]
}

vs

{
   method: 'someservice.get_dynamic_global_properties',
   params: {realparamsgohere}
}

on a method by method basis.

@jnordberg
Copy link
Contributor

We should never need to do request restructuring, here or in jussi. When is that needed?

As for not breaking backwards compatibility when we move some steemd calls to hive that will happen in jussi

@relativityboy
Copy link
Contributor Author

We should never need to do request restructuring, here or in jussi. When is that needed?
I agree with this in principle. If we have to switch between the "old format" and the new quickly (imagining some performance issue, outage, or bottleneck) we have a way of going back.

As for not breaking backwards compatibility when we move some steemd calls to hive that will happen in jussi

You're talking about the routing? My understanding is that condenser_api. and `` are supposed to point to steemd, even after hive goes live.

At some point in the nearish future we have a PR that let's us eliminate this type of call entirely.

{
   method: 'someservice.call',
   params: ['blah', 'get_dynamic_global_properties', {realparamsgohere}]
}

@roadscape
Copy link
Contributor

We need existing steemd APIs to remain accessible through jussi; we're not changing any paths, just adding the option to "upgrade" some calls to hive by adding a prefix.

Example:
get_followers will (continue to) go to steemd
condenser_api.get_followers will (continue to) go to steemd
hive.condenser_api.get_followers will go to hivemind

Right now hive's "condenser_api" compat layer request/response formats are 1:1 so that we can thoroughly test and easily revert to steemd should an issue arise.

In the future, we will move away from the "old" APIs to new hive methods which do not necessarily resemble steemd's at all.

@jnordberg
Copy link
Contributor

I'm with you on that @roadscape, my point is that we don't need the client-side method routing this introduces to support hive. All we need to do is use steem.api.call. E.g. switching between steem.api.call('condenser_api.get_followers', 'bar') and steem.api.call('hive.condenser_api.get_followers', 'bar'). Or am I missing something?

@relativityboy
Copy link
Contributor Author

Things have gotten a lot simpler since this issue (and the related PR) were filed.
Hive's MVP is to have one endpoint only, and Jussi is handling api structure translations.

I'l be revisiting the PR and this issue to see how much I can simplify it, possibly make it go away entirely.

@jnordberg philosophically you're spot on. That's definitely the simpler way for us to do it, and where we want to quickly get.

It also introduces more potential pain for users of the library.

Offering this configuration option gives us the ability to change all calls to api.getFollowers() of those hydrated functions from condenser_api.get_followers to hive.condenser_api.get_followers .. or any other hydrated function for that matter - from a single authoritative location. It also allows any 3rd party developers running our library to upgrade their library without having to do modifications to multiple locations in their code, rewrite tests, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants