Skip to content

First Java SDK version with usage example#3

Merged
vsbogd merged 52 commits intosingnet:masterfrom
vsbogd:get-payment-channel-state-from-daemon
Nov 21, 2019
Merged

First Java SDK version with usage example#3
vsbogd merged 52 commits intosingnet:masterfrom
vsbogd:get-payment-channel-state-from-daemon

Conversation

@vsbogd
Copy link
Copy Markdown
Member

@vsbogd vsbogd commented Nov 1, 2019

Class diagram and build instructions: https://github.com/vsbogd/snet-sdk-java/tree/get-payment-channel-state-from-daemon
Usage example: https://github.com/vsbogd/snet-sdk-java/tree/get-payment-channel-state-from-daemon/example

Changes included in PR:

  • Add PaymentProvider interface to provide payment for a client call;
  • Add organization related registry data structures;
  • Implement getting channel state from registry;
  • Add FixedPaymentChannelPayment strategy to pay for a call using predefined channel;
  • Add functional test for implemented functionality;
  • Handle payment state kept in daemon
  • Sdk and Configuration classes to simplify SDK bootstrapping
  • Add CntkImageRecognition service calling example
  • API refactoring
  • Test refactoring

Next steps:

  • compile for Android
  • develop maven plugin to simplify new app setup - plugin to download API from registry and modify it to use in Java
  • get rid of ./get_dependencies.sh script

vsbogd added 30 commits October 18, 2019 17:30
In fact autogenerated protobuf code generates such warnings which lead
to the red CI status.
@vsbogd vsbogd requested a review from raamb November 1, 2019 15:09
Copy link
Copy Markdown

@keshrisohit keshrisohit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@vsbogd
Copy link
Copy Markdown
Member Author

vsbogd commented Nov 5, 2019

@keshrisohit , you can look at code coverage report here

I used functional tests for main business cases, and unit tests to test pieces which is impractical to cover by functional test.

@vsbogd
Copy link
Copy Markdown
Member Author

vsbogd commented Nov 8, 2019

@keshrisohit , @raamb , do you have any questions on the PR? I would like to merge it and move forward. We can setup a call to discuss if you need it.

@raamb
Copy link
Copy Markdown
Contributor

raamb commented Nov 9, 2019

@vsbogd have requested Chetan to review as well. I am good from my side for the first merge

Comment on lines +31 to +34
Sdk sdk = new Sdk(config);
try {

PaymentStrategy paymentStrategy = new FixedPaymentChannelPaymentStrategy(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python and js sdk currently takes paymentStartegy as sdk input parameter not as service_client parameter , if it makes sense all three sdk should have common api.

Copy link
Copy Markdown
Member Author

@vsbogd vsbogd Nov 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that all three SDK should use same API. But Python and JS SDK takes payment strategy as parameter of create_service_client method, see links below:
Python SDK: https://github.com/singnet/snet-cli/blob/eca41c7e0549fc6bc5aef5a09c18495893fb378f/packages/sdk/snet/sdk/__init__.py#L58-L66
Node.js SDK: https://github.com/singnet/snet-sdk-js/blob/a315f7e95da0e39ac436ed11aff417d3224d70d0/packages/nodejs/src/NodeSdk.js#L14-L18
Web SDK: https://github.com/singnet/snet-sdk-js/blob/a315f7e95da0e39ac436ed11aff417d3224d70d0/packages/web/src/WebSdk.js#L13-L17

I believe it is correct solution as each ervice may have different payment methods etc, so it may require different strategies.

Copy link
Copy Markdown

@keshrisohit keshrisohit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could not locate channel apis like opne_channel , deopsite_and_open_channel ,load_open_channels

Copy link
Copy Markdown
Member Author

@vsbogd vsbogd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't implement them in this PR, because I was focused on simplest possible but handy strategies implementations. My opinion the simplest payment strategy is using predefined payment channel. Client can initialize it once and extend it when it is required manually. I have plan to implement these methods later though.

@vsbogd
Copy link
Copy Markdown
Member Author

vsbogd commented Nov 21, 2019

Ok, guys, three weeks on review stage, I am merging it.

@vsbogd vsbogd merged commit e421271 into singnet:master Nov 21, 2019
@vsbogd
Copy link
Copy Markdown
Member Author

vsbogd commented Nov 21, 2019

@keshrisohit , I propose raising issue and add your comments to it, as this PR is already stale. I am going to raise new one in nearest time.

@vsbogd vsbogd deleted the get-payment-channel-state-from-daemon branch December 19, 2019 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants