fabric-gateway icon indicating copy to clipboard operation
fabric-gateway copied to clipboard

Migrate fabric-samples to use Gateway SDKs

Open jt-nti opened this issue 3 years ago • 19 comments

The following samples should be extended with gateway app samples. We don't need to have a full matrix of all samples in a languages however. Items with a 'P' are initial priorities for this work item. Add your name next to an item if you intend to pick it up.

Higher priority

  • asset-transfer-basic (demonstrates transaction submit / evaluate, submit error capture)
    • [x] Typescript - P1 - Saptha
    • [x] Java - P2 - Deepti
    • [x] Go - P2 - Saptha
  • asset-transfer-events (demonstrates chaincode events)
    • [x] Typescript - P3 - Saptha
    • [x] Java - P3 - Deepti
    • [x] Go - P3 - Mark
  • asset-transfer-private-data (demonstrates auto-magic handling of endorsement with private data collections)
    • [x] Typescript - P4 - Saptha
    • [ ] Java - P4
    • [ ] Go - P4
  • asset-transfer-secured-agreement (demonstrates auto-magic handling of state-based endorsement)
    • [x] Typescript - P4 - Saptha
    • [ ] Java
    • [ ] Go
  • off_chain_data (demonstrates block eventing, checkpointing)
    • [x] TypeScript - Mark
    • [x] Java - Mark
    • [ ] Go
  • full-stack-asset-transfer-guide trader application
    • [ ] Go
    • [ ] Java

Lower priority

  • asset-transfer-ledger-queries
    • [ ] Typescript - P5
    • [ ] Java
    • [ ] Go
  • asset-transfer-sbe
    • [ ] Typescript - P5
    • [ ] Java
    • [ ] Go

What about other samples? For example the old fabcar samples and commercial paper?

jt-nti avatar Sep 14 '21 10:09 jt-nti

Unless other samples are demonstrating API usage not already covered by the asset-transfer samples, I vote we skip them. In fact, I think we'd be better actively removing the legacy samples from the main branch and just leave them in the older release branches. They just cause confusion for end users.

bestbeforetoday avatar Sep 22 '21 17:09 bestbeforetoday

Should this issue also include removing the samples included in this repository? Maintaining one set of samples seems like a better use of resources and less confusing for users. We would need to be sure that there is still a sample demonstrating features like off-line signing (if there isn't already one in fabric-samples).

bestbeforetoday avatar Sep 22 '21 17:09 bestbeforetoday

@denyeart Could you please assign the issue to me?

deeptiraom avatar Nov 17 '21 07:11 deeptiraom

Currently I am working on the below:

asset-transfer-basic Java

deeptiraom avatar Nov 17 '21 07:11 deeptiraom

I have done the following so far:

  • created a directory which mirrors application-java under asset-transfer-basic folder
  • made changes to App.java using the samples under fabric-gateway
  • Need to make further changes to EnrollAdmin and RegisterUser understanding the code in fabric gateway samples
  • as a next step I will build the code with all changes

deeptiraom avatar Dec 01 '21 12:12 deeptiraom

@deeptiraom In the typescript sample for gateway, we decided to simply re-use the test-network credentials instead of Enroll/Register new credentials. We could do the same for Java. See https://github.com/hyperledger/fabric-samples/blob/main/asset-transfer-basic/application-gateway-typescript/src/app.ts

denyeart avatar Dec 01 '21 13:12 denyeart

@denyeart I have completed the basic-asset-transfer-basic go samples migration.

sapthasurendran avatar Dec 15 '21 12:12 sapthasurendran

I am picking this up again after a weeks training from Dec 13-Dec 16

deeptiraom avatar Dec 17 '21 10:12 deeptiraom

@bestbeforetoday To respond to your earlier Sept 22 points, I think this is where we landed:

  • Remove the existing samples from fabric-gateway repo (these were used during initial alpha/beta dev only)
  • The new asset transfer gateway SDK app samples in fabric-samples will live next to the legacy SDK asset transfer app samples, but all docs, tutorials, etc should point people to the new gateway app samples only.
  • Eventually the old asset transfer app samples should be deleted (maybe 1 year co-existence?)
  • Other outdated samples such as fabcar and everything in /chaincode directory should be deleted even sooner, since we've already had about 1 year overlap with the newer asset transfer samples.
  • Developing Applications doc that utilized Commercial Paper will be removed and/or revamped as part of https://github.com/hyperledger/fabric/issues/2983 / https://github.com/hyperledger/fabric/issues/3157 , but keep Commercial Paper sample itself as a good industry example.

What do you think @bestbeforetoday @lehors ?

denyeart avatar Feb 03 '22 18:02 denyeart

Thanks for giving me a chance to chime in on this @denyeart .

I agree with the overall gist of this but rather than having the new gateway samples be added under a new name as in "asset-transfer-basic/application-gateway-go" I would much rather rename the old one to something like "asset-transfer-basic/application-legacy-go" and put the new version under the old name.

This is so that: 1) it is clear what samples people should really follow, 2) when we eventually get to retire the legacy stuff we don't have all these names with "-gateway-" in them.

I realize that some of the new stuff has already been added under the new names but this can easily be fixed. I'm happy to submit a PR if that helps.

lehors avatar Feb 03 '22 19:02 lehors

Could we take advantage of the fact that the fabric-samples repository has branches for each release? Maybe we could update the top-level README to point people to the branch appropriate for their Fabric release, and also mention that the main branch corresponds to either the latest Fabric release (v2.4) or v.next. We could then just go ahead and delete any samples that have been superseded or are no longer relevant for the current release, safe in the knowledge that people using older versions can still access them in other branches. What do you think?

This approach might benefit from an update to the Fabric install script to flip to the fabric-samples branch corresponding to the installed Fabric version after cloning the fabric-samples repository.

I definitely agree with removing the samples from the fabric-gateway repository once we have sufficient Go, Node and Java samples in fabric samples to cover all the usage demonstrated here. One of the key selling points of the Fabric Gateway client API is completely consistent functionality and behaviour in all client languages so we should cover all three languages in all Fabric Gateway samples.

I like the idea of changing the application-gateway- samples to just application-, and renaming (or removing) the legacy samples in the main branch. Just decide whether rename or remove is preferred and we can start doing that straight away.

bestbeforetoday avatar Feb 04 '22 11:02 bestbeforetoday

Having separate release branches for fabric-samples sounds good in theory, but it comes with additional maintenance burden, sometimes much more. It explodes the number of assets that have to be managed, for every PR we'd have to consider if it was worth backporting or not, and we'd end up with some samples demonstrating the latest improvements and good practices and some not. My opinion has been to only create release branches for major incompatibilities, such as system channel removal (release-2.2 branch is pre-removal and you get it if you pull down Fabric v2.2.x, main branch is post-removal and you get it if you pull down any later version of Fabric that comes with osnadmin).

Besides lower maintenance, another good aspect of keeping samples on main branch for the last few releases is that people will always get the latest and greatest samples.

Yes, this approach also has some downsides, for example if you pull down Fabric v2.3 it won't work with the new gateway apps (although the v2.3 docs won't mention the gateway apps anyways). I didn't think that was enough of an incompatibility to warrant additional release branch maintenance, especially given that v2.3 is not a LTS release nor a latest release and therefore I don't expect too many people to pull it down. But this to me is a reason to keep the legacy vs gateway assets distinct with distinct names. I didn't mind having 'gateway' in the name since we call it the 'Gateway API' / 'Gateway SDK' in our docs so it actually maps rather nicely. I think it helps users know which is which, and helps us keep our sanity as maintainers.

I would be fine putting the name 'legacy' in the old ones to give people a clue and retiring them sooner than later (although it requires updating the Fabric docs/tutorials in prior release branches...these changes always have more ripple than you expect). This doesn't go as far as you guys were thinking but is somewhat of a middle ground. WDYT now given the explanation @lehors @bestbeforetoday ?

denyeart avatar Feb 04 '22 20:02 denyeart

Although I like @bestbeforetoday's idea of using branches in principle I have to agree with @denyeart that this is probably too much of a pain to manage in practice. I'm fine with the proposed middle ground approach. It seems reasonable to me. Thanks.

lehors avatar Feb 07 '22 13:02 lehors

I'm not really sure what additional maintenance is required. We already have a release-2.2 branch to be maintained. I'm not sure why removing samples from main increases the maintenance burden. It seems like it should reduce it since maintenance on the samples in release-2.2 now only needs to be done in that branch and not in main. Can you elaborate?

The rename of existing samples may have a potential maintenance impact. Changes to legacy samples in main branch may be harder to cherry-pick back to previous branches once names diverge. Having said that, if things are working in release-2.2, they shouldn't stop working unless we've introduced a regression, so just leaving them alone and doing no maintenance on them should be fine.

Either way, whatever we can do to make people more likely to use the new samples and not the old ones is a win. If just renaming the samples is as far as you want to go to achieve that, that's fine by me. Shall we go ahead and do that renaming now?

One more question... what do we do with all the samples that use the legacy SDKs but don't have an equivalent using the Fabric Gateway client API? Do they just stay as-is or get renamed to "legacy" too?

bestbeforetoday avatar Feb 11 '22 11:02 bestbeforetoday

On the similar lines with @bestbeforetoday, does this also mean that the Wallet API will be marked as legacy? And what's the alternative for it?

HandOfGod94 avatar May 03 '22 13:05 HandOfGod94

@HandOfGod94 The Fabric Gateway client API uses a simplified (and more abstract) mechanism for dealing with client identity and signing credentials, and doesn't include an equivalent to Wallets. If you want, you can continue to use the wallets from the older SDKs to store credentials. See the migration guide for more information:

https://hyperledger.github.io/fabric-gateway/migration#wallets

bestbeforetoday avatar May 03 '22 15:05 bestbeforetoday

Hello @denyeart, @bestbeforetoday, I'd like to work on this issue. I've reviewed the details and can start immediately.

Sayalikukkar avatar May 20 '24 03:05 Sayalikukkar

@Sayalikukkar thank you very much for the interest, and apologies for not getting back to you more quickly. It would be great if you can contribute any of the missing sample applications using the Fabric Gateway client API mentioned in the description of this issue. I would recommend following the structure of the existing (gateway) samples, and ensure that the flow and console output is consistent between language implementations.

I think the most important ones are:

  • Samples where we have a legacy SDK implementation but no Fabric Gateway client API implementation.
  • off_chain_data (Go).
  • full-stack-asset-transfer-guide (Go, Java).

Let me know if you have any questions.

bestbeforetoday avatar Jun 07 '24 16:06 bestbeforetoday

Hello @bestbeforetoday Thank you for your guidance. I would like to start working on off_chain_data with golang and will follow the structure existing gateway samples. If I encounter any questions or need further clarifications, I'll reach out promptly.

Thanks!

Sayalikukkar avatar Jun 08 '24 06:06 Sayalikukkar