SDK API discussion #16
Comments
|
I would like to welcome anyone to share your thoughts on both items. |
|
@vsbogd the session concept sounds quite interesting. If I were to extend your example to interact with two different services as shown below, is the session object expected to manage channels separately? |
For example statically compiled example of @vsbogd will look like this: import snet
import snet.example_service as example_service
# load configuration which contains:
# - current ethereum identity
# - current network (with all information: rpc_endpoint, default_gas_price_strategy and optional contract_addresses)
# - ipfs endpoint
# (SDK specific) - caching strategy
# (SDK specific) - "is_allow_transaction" and others..
config = snet.config("/path/to/your/config")
# we need to pass <org_id> <service_id> here, or hardcode it in "snet.example_service"
example_service = example_service.Calculator(config, "<org_id>", "<service_id>")
# the rest is like in @vsbogd example
....We might also want to support dynamically compiled calls: import snet
import snet.call_stub_generator as call_stub_generator
config = snet.config("/path/to/your/config")
stub = call_stub_generator("snet", "example-service", "add")
# parameters could be a dictionary, I don't think that we need Request class here
params = {"a":1, "b":2}
rez = stub(params) |
|
What will people use if they are testing their service and want to use the same code path, but making grpc requests without the extra SNet headers? This is why I suggested matching the grpc API, unless we are planning to do major API work for every programming language and want to force people to learn our new API when the grpc API already exists. The alternative is the dynamically compiled calls that Sergey has already implemented. It is perfect for ease of use when people are prototyping an integration with an SNet service. I'm not sure a halfway point between fully dynamic and fully grpc API is useful.
import snet.example_service as example_service
Rereading the code example I see the mention of running a CLI command to generate the stub. This makes sense and will be required for any compiled language anyhow. However, there are then these questions:
If we do this anyway, I think we should namespace services from the rest of our sdk |
|
After thinking about this, I realise the session object could be configured in a way to remove all the SNet stuff and preserve the same code path when people are testing their service outside of SNet. Something like: session = DirectSession(endpoint="127.0.0.1:8080")which could then be passed to the stub creator. The question about how much we should diverge from standard grpc/protobuf API still remains though. |
|
If we use a Session object, I would prefer us not to go overboard with argument classes. It makes sense for identity, but there is no need to make ipfs and network setup an object, just make them string arguments to the session. If we want to have defaults that are easy to use instead of remembering endpoints, provide those as class or module constants: e.g. session = Session(identity=..., ipfs=snet.DEFAULT_IPFS, network=snet.KOVAN)
# of course, if these are the default, then it just becomes:
session = Session(identity=...)Or (Sorry for three comments in a row!) |
|
@ferrouswheel config = snet.config("/path/to/your/config")
# or it might be like this:
# config2 = snet.static_config(identity=, ipfs=...,network, buch_of_other_parameteters=...)
session = session(config=config)However here a session is actually a state of current configuration. I'm not sure that we need to initialize something in the session (it means that we might not need an abstraction for session, but of course we might keep it... ) |
|
I'm not fussed about session vs config. I'd prefer fewer things to set up, but I don't know enough to say whether a session is necessary... Sorry, I didn't add all the parameters to try to keep it simple... everything that has a sensible default should use the sensible default. What are the bare minimum arguments required that we can't assume? As far as I can tell it's network and identity. Anyhow, I was primarily wanting us to avoid |
Exactly, we need to be very careful. Even if we go with the session approach, I would prefer to have channels called out as an explicit entity. The channel concept is very important to call out even if we simplify it. |
|
After playing with current SDK and trying to merge it with functions from snet-cli I came the the following API: I propose to have two types of clients:
It should be noted that:
You should get this branch to run the following examples: https://github.com/astroseger/snet-cli/tree/new-sdk (new-sdk branch in my repo!). Example for from snet_cli.sdk import Session, BasicClient, AutoFundingClient
from snet_cli.config import Config
# we directly use snet-cli configuration
# another types of configuration can be easiliy implemented (for example hard-coded configuration)
session = Session(Config())
# we automatically open channel if needed
# each time we run out of funds we add 10 cogs
# each time we run out of "expiration" we add 20 days
client = AutoFundingClient(session, "snet", "example-service", "add", 10, "+20days")
for i in range(1000):
request = client.request_class(a=20, b=i)
rez = client.stub.add(request)
print(rez)Example with from snet_cli.sdk import Session, BasicClient, AutoFundingClient
from snet_cli.config import Config
session = Session(Config())
# we automatically open channel if needed!
# add 10 cogs to snet/example-service escrow
session.reserve_funds("snet", "example-service", 10, "+100days")
client = BasicClient(session, "snet", "example-service", "add")
for i in range(1000):
unspent_amount = client.unspent_amount()
if (unspent_amount < 1):
session.reserve_funds("snet", "example-service", 10, "+100days")
request = client.request_class(a=20, b=i)
rez = client.stub.add(request)
print(rez) |
We should not force the client to be dependent on the method called. When people start providing richer APIs (beyond a single request/response method) it will be annoying to create a new client for every method.
Can you include an example of how you would use plain GRPC? Are all the request and response protobuf classes available via the client? What happens if their API names clash with client property names? |
|
@ferrouswheel
What exactly do you want? If you want to test your service before registered it then I have the question to your: Do you want to do it with or without a daemon? If you want to test your service without the daemon (so without any channels and payments), then you can directly use grpc. If you want I can add a class with the same syntactic sugar... Something like this: # ATTENTION!!! this example will not work, it is not implemented yet
from snet_cli.sdk import TestClient
client = TestClient("/path/to/your/compiled/protobuf/files/", "add")
for i in range(1000):
request = client.request_class(a=20, b=i)
rez = client.stub.add(request)If you want to test your service with a daemon (but without registered it in Registry). Then we have mechanism in snet-cli to initialize service directly from metadata (@raamb preventing me to remove this mechanism recently :) ). So you initialize your service and directly use |
|
There is more complexity to grpc and protobuf messages than just request
and response classes, and those will become far more obvious outside of
python (in any staticly typed language).
And as I said. I really think making clients dependent on the specific
method is a bad idea. If you were creating an http request library, you
wouldn't create a new client for every http url!
…On Mon, 11 Mar 2019, 9:40 PM Sergey Rodionov, ***@***.***> wrote:
@ferrouswheel <https://github.com/ferrouswheel>
It is plain grpc, because:
- client.stub is proper grpc stub
- client.request_class is proper grpc request_class
- (if you need it) client.response_class is a proper grpc response
class
Can you include an example of how you would use plain GRPC?
What exactly do you want?
If you want to test your service before registered it then I have the
question to your: Do you want to do it with or without a daemon?
If you want to test your service without the daemon (so without any
channels and payments), then you can directly use grpc. If you want I can
add a class with the same syntactic sugar... Something like this:
# ATTENTION!!! this example will not work, it is not implemented yetfrom snet_cli.sdk import TestClient
client = TestClient("/path/to/your/compiled/protobuf/files/", "add")
for i in range(1000):
request = client.request_class(a=20, b=i)
rez = client.stub.add(request)
If you want to test your service with a daemon (but without registered it
in Registry). Then we have mechanism in snet-cli to initialize service
directly from metadata ***@***.*** <https://github.com/raamb> preventing me
to remove this mechanism recently :) ). So you initialize your service and
directly use BasicClient or AutoFundingClient. I want to underline that
if your service is Registered in Registry then all initialization is done
automatically on fly. Of course without registration we cannot do it on fly.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#16 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHWB1SZoWExFDKSEwPZice1ydf40ekNks5vVhaNgaJpZM4bilJ6>
.
|
|
Extending the idea @astroseger proposed here about multiple clients, with and without auto funding. Rather than having two different clients we could rather have one client and allow the user change the funding strategy and we could provide a default strategy which the user needs to explicitly connect to the client. Something along these lines (extending @astroseger's example) from snet_cli.sdk import Session, Client, DefaultFundingStrategy
from snet_cli.config import Config
# we directly use snet-cli configuration
# another types of configuration can be easiliy implemented (for example hard-coded configuration)
session = Session(Config())
# we automatically open channel if needed
# each time we run out of funds we add 10 cogs
# each time we run out of "expiration" we add 20 days
client = Client(session, DefaultFundingStrategy(), "snet", "example-service", "add", 10, "+20days")
for i in range(1000):
request = client.request_class(a=20, b=i)
rez = client.stub.add(request)
print(rez)If the user wants to define her own funding strategy we could define a contract which a strategy needs to adhere to. This then opens up possibilities for a lot of different strategies the user can define without changing the client. If we think this is a good path to take then we could work towards refining this further. |
|
I don't think that specifying the name of protobuf file (like in @vforvalerio87 SDK) is a right path. Sure It is possible to do the same, but I think it is not the best we could do... What about API in which you specify the name of protobuf-service (in case of multiply protobuf services )? For example: from snet_cli.sdk import Session, Client, AutoFundingFundingStrategy
from snet_cli.config import Config
# we directly use snet-cli configuration
# another types of configuration can be easiliy implemented (for example hard-coded configuration)
session = Session(Config())
# In case of multiply grpc_service names you should specify the name of grpc_service
# Here we have only one grpc_service (Calculator), so we can omit this parameter
client = Client(session, "snet", "example-service", AutoFundingFundingStrategy(10, "+20days"))
for i in range(1000):
# unfortunately you must know the name of request_class
request = client.classes.Numbers(a=20, b=i)
rez = client.stub.add(request)
print(rez)
rez = client.stub.mul(request)
print(rez)
rez = client.stub.sub(request) |
|
@chetankothari |
|
@astroseger I think the combination of your TestClient idea and having the protocol buffer classes available in However, we'd also need to figure out how to handle multiple proto files, since you could have the same pb message name in different files, so collapsing them all into one namespace is going to cause trouble eventually. e.g. if I had This is one of many reasons for trying to make the core SDK as close to pb/grpc as possible which already handles all of these edge cases. One other reason is that we'd need to implement our own code generation for C++/C/Rust SDKs, on top of what pb/grpc provides, if we wanted our SDKs to be similar across languages. I originally liked your "simple" SDK idea as a separate interface which is specialised to Python and would work for 90% of service develoeprs, but with a core SDK would expose the pb/grpc details for people that needed it. |
@astroseger by contract @chetankothari was referring to the API interface contract and not a blockchain contract. |
@ferrouswheel , I believe multiple protobuf files problem should be resolvable via protobuf packages. In your example even protobuf compiler should complain when you use the same BoundingBox name to define two different messages and include them in driving.proto. To solve this one can use different I think we need compile protobuf supporting packages and adding some prefix package as you suggested here. For instance For example service it would be |
Answering @astroseger comment here. In my example class ExampleService:
def __init__(self, session):
self.session = session
self.org_id = "example-organization"
self.service_id = "example-service"
def add(self, request):
# below are pseudo-code steps we need to execute to call method
md = formGrpcMetadata("add", request)
response = self.grpc_stub().add(request, metadata=md)
return responseTo generate this code from protobuf service description I thought about developing protobuf compiler plugin. |
|
Regarding channel management API I see two main controls we should provide to API user:
Payment address and channel are bounded to service replicas group. So I think both group_id and payment channnel strategies should be parameters of the service instance constructor. |
|
Answering @ferrouswheel comments here
In my example generated service API is absolutely identical to gRPC once the only difference is the way of "gRPC channel"/"Snet session" initialization. I think the most simplest way is to switch between plain gRPC and snet infrastructure is to replace line: example_service = example_service.Calculator(session, channel_update_strategy, channel_caching_strategy)by line: example_service = example_service_pb2_grpc.ExampleServiceStub(channel)and vice versa.
Generated service code should do this work it should download service metadata and make all necessary checks.
As I wrote here, I think we need to generate different python packages for them and then use fully qualified names for classes.
I think having them outside will allow us testing same service in different environments.
We can translate them as
Agree it is a good option. |
|
We have two slightly different threads above:
Thanks to Python abilities they doesn't look very different. But for statically compiled languages without reflection like Dynamically generated API suggested by @astroseger is based on using plain protobuf compiler with grpc plugin. But it also can be based on using snet sdk compiler which is protobuf compiler with snet-sdk plugin. In such case dynamic API incorporates static API and static approach is reused to implement dynamic one. |
My issue with this is that the service could have a completely different API in different environments. I think that once the API is generated for a given environment, it should be bound to it. Or at least calling a kovan service with a mainnet session should raise an error if the service has different metadata. (this only applies to the |
SDK API design proposal (Dynamic languages)I think we have great ideas throughout the thread and taking inspiration from most of them here is an API design that I and @raamb have come up with: The scope of this API proposal is for building an SDK for AI service consumer (client). We could re-use existing code from snet-cli if needed by extracting them out into a common package. The high level api looks something like this. import snetSdk from 'snet-client-sdk';
import { DefaultChannelManagementStrategy, DefaultChannelSelectionStrategy } from 'snet-client-sdk';
const sdk = snetSdk.init(configFilePath);
sdk.account.depositToEscrowAccount(0.00000001);
const defaultChannelSelectionStrategy = new DefaultChannelSelectionStrategy();
const defaultChannelManagementStrategy = new DefaultChannelManagementStrategy(defaultChannelSelectionStrategy);
const serviceClient = sdk.createServiceClient('org-id', 'service-name', defaultChannelManagementStrategy);
const addRequestData = {a: 1, b: 2};
const addResponse = serviceClient.stub.add(addRequestData);
const multiplyRequestData = {a: 3, b: 6};
const multiplyResponse serviceClient.stub.mul(multiplyRequestData);
class ChannelManagementStrategy {
setup(mpeContract, serviceClient, account, tokens)
}The class DefaultChannelManagementStrategy extends ChannelManagementStrategy {
constructor(channelSelectionStrategy) {
this.channelSelectionStrategy = channelSelectionStrategy;
}
setup(mpeContract, serviceClient, account, tokens) {
// pseudo code
// get all the channels
// use channel selection strategy to pick a channel for performing the operation (this could be done by either using the ChannelSelectionStrategy abstraction or however the developer would like it to. Since we would be giving default channel management strategy we could rely on ChannelSelectionStrategy)
// create channel no channel found
// fund channel if insufficient banance with appropriate tokens
}
}
// Note: the extends here just signifies that it follows ChannelManagementStrategy contractclass ChannelSelectionStrategy {
get select(channels)
}
class DefaultChannelSelectionStrategy extends ChannelSelectionStrategy {
get select(channels) {
// pseudo code
// return null if channels list is empty
// pick a channel with max unsued funds and return the channel id
}
}The general idea of adding
The statically typed languages could use the same api but with just an additional parameter towards the end, which could be the generated This is the simplest use case and we need to define the complete set of methods and operations that could be possible with the sdk. Once we have a high level API defined we can get started implementing it and iterate on it as we get more clarity. |
@ferrouswheel , could you please explain this case further? Do you mean that you usually have two different service API versions published in different Ethereum network? Like you have service release version published in Mainnet and some work-in-progress version of API published in local ethereum or Kovan for debugging purposes? I am not sure if scenario I have described above is a big issue as:
|
|
On this: most developers would much rather not have to think about channels. They will want us to provide them with a default approach that is sensible so they can focus on feature development, not payment handling. So I think we should spend our time figuring out how to make the default way to use the SDKs reflect that preference. Making it configurable doesn't need to be there in v1 and there's plenty of higher priority issues.
|
|
@pennachin we will be seeding defaults for all these options so that the developer can use the SDK without having to configure much. However we do want to have the provision in the SDK to customise this in future, given that once we release the SDK we need to worry about backward compatibility we want to ensure that the first approximation is as good as possible. |
|
@vsbogd @astroseger @ferrouswheel @vforvalerio87 please share your thoughts on the API, if nothing is amiss we can start implementation in respective languages. |
|
@chetankothari , @raamb , few remarks on design example you have posted: (1) In following code snippet from your example const sdk = snetSdk.init(configFilePath);I would prefer have a choice between providing configuration in a code itself or load it from external storage. So I would instead introduce configuration interface (or class) and pass it to configuration = new Configuration();
configuration.setIpfsEndpoint(...);
configuration.setEthereumEndpoint(...);
const sdk = snetSdk.init(configuration);Also I think we need to list which parameters are kept in configuration, at list major one. (2) I think we should add To be clear interface ChannelStateProvider {
ChannelState getState(BigInt channelId)
}
(3) I don't understand whether we really need this |
|
I have tried to draw UML diagram for design we have right now: You can see source code and edit it here |
Makes sense, because this could allow clients running on machines with no access to filesystem. Listing the configuration needed for the sdk to work.
As mentioned in the list we only need the identity to as mandatory and we could have defaults for the rest of them.
Yes, your are right we were also thinking along similar lines about
This was just to simplify the part where we have to dynamically add methods to the |
|
Hi @vsbogd here is a simplified version of the diagram that you had. We focused on how the SDK design would look like. Essentially we are still looking at a ChannelManagementStrategy which will compose ChannelSelectionStrategy, ChannelUpdateStrategy and ChannelStateProvider. We are looking at keeping the abstraction at the ChannelManagementStrategy level for the SDK but the implementation would use the classes you mentioned. Again the idea is to get started with a fairly good approximation as the first step and then iterate over it.
I feel that we have a pretty good high level API to start implementing with. We will have additional discussions where necessary as we proceed with the implementation. |
@raamb , @chetankothari , I agree that essentially we can start from this. |
|
I think that couple of interfaces should be enough to provide user an API to manage channel state: One is to update channel state: class PaymentChannel {
Transaction addAmount(amount);
Transaction extendExpiration(expiration);
}
class Transaction {
Result get(timeout);
cancel();
State state();
}SDK could return sdk.paymentChannel(channelId).addAmount(0.001);
sdk.paymentChannel(channelId).extendExpration(...);And second is to listen channel state updates: class ChannelListener {
onUpdate(ChannelState old, ChannelState new);
}Then client can implement channel listener and subscribe to updates from channel provider to implement any channel funding strategy. |
|
I have updated diagram above:
The diagram: |
|
Sharing the result of discussion with @vforvalerio87 , @raamb and @astroseger. We have discussed three ways to implement the SDK:
We have agreed on using approach (1) to implement SDK, it seems most simple and powerful enough to implement metadata injection and choose endpoint. Other ways can be used in case gRPC API is not enough to do all SDK related work. |
|
It will be a little bit complicated so please follow me. As I see in API which proposed @chetankothari for js there is the following line: const addRequestData = {a: 1, b: 2};
const addResponse = serviceClient.stub.add(addRequestData);So he passes parameters as a "dictionary", not as request_class. Which actually makes a lot of sense for me, because it seems there is not advantage in passing parameters as request_class in python, because, in any case, everything will be validated inside the stub. It is especially true in complicated cases! Let's consider a slightly complicated example with message contains another messages. Let us have example service with the following protobuf (message Numbers contains message Number) syntax = "proto3";
package example_service;
message Number {
float x = 1;
string name = 2;
}
message Numbers {
Number a = 1;
Number b = 2;
}
message Result {
Number value = 1;
}
service Calculator {
rpc add(Numbers) returns (Result) {}
rpc sub(Numbers) returns (Result) {}
}We can have two variants of python SDK API.
Please note that you actually have two alternatives regarding nested messaged here. You can pass nested message as a dictionary and it will work! from snet_cli.sdk import Session, Client, AutoFundingFundingStrategy
from snet_cli.config import Config
session = Session(Config())
client = Client(session, "snet", "example-service", AutoFundingFundingStrategy(10, "+20days"))
# V1.1: using Number and Numbers classes
a = client.classes.Number(x=20, name="a")
b = client.classes.Number(x=10, name="b")
request = client.classes.Numbers(a=a, b=b)
rez = client.stub.add(request)
# V1.2: using only Numbers classes (yes! it will work in grpc python automatically)
request = client.classes.Numbers(a={"x":20, "name":"a"}, b={"x":10, "name" : "a"})
rez = client.stub.add(request)
from snet_cli.sdk import Session, Client, AutoFundingFundingStrategy
from snet_cli.config import Config
session = Session(Config())
client = Client(session, "snet", "example-service", AutoFundingFundingStrategy(10, "+20days"))
rez = client.stub.add({"a" : {"x":20, "name":"a"}, "b" : {"x":10, "name" : "a"} })which variant of API do you prefer? |
|
@Sergey For dynamic languages I think it's fine to allow dictionary as request parameters, so long as we can still pass the original message objects. For example, if you call one service e.g. The client should be able to easily chain data from one service to the next, without extra conversion steps (even if they are implicitly handled by the SDK, it is extra overhead/latency that is unnecessary) BTW - nested messages, represented as a dictionary, already works with snet-cli when I call my services. So maybe it's supported by grpc python already? (or you added that feature ;-) ) |
|
@ferrouswheel # Numbers is a request_class for client_stub
# client_stub is grpc client stub
request = Numbers(a = {x:10, "name" : "a"}, b = {x:20, "name": "b"})
client_stub.add(request)But it seems there is no build in mechanism for passing full message as a dictionary (you must have a request_class)... or is it? request = {"a" : {x:10, "name" : "a"}, "b" : {x:20, "name": "b"}}
# this will not work :(
client_stub.add(request)
# It seems the only one way is to convert to request_class by hand
client_stub.add( Numbers(**request) )But in any case I do agree with you that we should allow passing request_class (not only a dict). Ok as a first demonstration I would prose the following API, by default we pass request_class, but we have mechanism for passing dictionary without knowing the name of request_class for example (in context of #16 (comment)): request_dict = {"a":{"x":20, "name":"a"}, "b":{"x":10, "name" : "a"}}
rez = client.stub.add(client.get_request_class("add")(**request_dict)) |
|
@astroseger the approach of supporting both message object and dictionary sounds good to me |
|
I've implemented python-SDK which fully comply to discussed API (branch new-sdk on my fork on snet-cli https://github.com/astroseger/snet-cli/tree/new-sdk). @vforvalerio87 you could reuse I want to note that it has the following features:
You can tests it by yourself, everything is functional from snet_cli.sdk import Session, Client, AutoFundingFundingStrategy
from snet_cli.config import Config
# we directly use snet-cli configuration
# another types of configuration can be easiliy implemented (for example hard-coded configuration)
session = Session(Config())
# In case of multiply grpc_service names you should specify the name of grpc_service
# Here we have only one grpc_service (Calculator), so we can omit this parameter
client = Client(session, "snet", "example-service", AutoFundingFundingStrategy(amount_cogs = 4, expiration = "+10days"))
for i in range(10):
request = client.classes.Numbers(a=20, b=i)
rez = client.stub.add(request)
print("add", rez)
request = client.get_request_class("mul")(a=20, b=i)
rez = client.stub.mul(request)
print("mul", rez) |
|
I have redrew the diagram incorporating results of our today discussion.
|
|
Looks great @vsbogd. I have further added more details to individual components and tried to simplify some components. One major change we will see is the
|
|
@chetankothari @vsbogd Quick thoughts:
Minor spelling correction in the Class Name: DinamicClassProvider -> DynamicClassProvider |
|
@vforvalerio87 @vsbogd @astroseger please review and sign off on the API. We can start to building this out right away |
|
@vforvalerio87 @vsbogd @astroseger havent received any comments so take it that we are all good with the API proposed in #16 (comment) |
|
@chetankothari, @raamb, sorry for a late response. My comments to the questions of @chetankothari above:
Not sure what is PWD but list of
Yes, actually I think all this stuff should have some root package namespace to separate it from other software.
I don't think we should abstract Organization. I didn't add it to the diagram yet as I think it is not so important for the first SDK version and I would not like thinking about all things at once. We can add some separate method to work with organization to the SDK later. |
|
Major comments on
|
|
@vsbogd I mean Json Files with password as one of the Identity along with the existing identities we have. |
|
@vsbogd @vforvalerio87 @astroseger @ferrouswheel @raamb JS SDK mostly done and minor things are pending. Since this is a huge PR, it would be great if you guys could add your feedback on the PR. |
I have created this issue to discuss the API which should be provided by SDK for client development.
Currently implemented approach is to generate client API stubs on the fly when new client instance is created (see Usage section of the README.md).
Example I have posted below has two items which I would like to discuss:
Item 2 is not so important probably for interpreted languages like
PythonorJavaScriptbut it can be faster and may be simpler than generating stubs on the fly. BTW it is harder to reuse such stubs insnet-cliso it is a bit contradicts to issue #9.The text was updated successfully, but these errors were encountered: