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

Migrate fabric-samples to use Gateway SDKs #190

Open
11 of 23 tasks
jt-nti opened this issue Sep 14, 2021 · 20 comments
Open
11 of 23 tasks

Migrate fabric-samples to use Gateway SDKs #190

jt-nti opened this issue Sep 14, 2021 · 20 comments
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers hacktoberfest https://hacktoberfest.com/ help wanted Extra attention is needed
Milestone

Comments

@jt-nti
Copy link
Member

jt-nti commented Sep 14, 2021

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)
    • Typescript - P1 - Saptha
    • Java - P2 - Deepti
    • Go - P2 - Saptha
  • asset-transfer-events (demonstrates chaincode events)
    • Typescript - P3 - Saptha
    • Java - P3 - Deepti
    • Go - P3 - Mark
  • asset-transfer-private-data (demonstrates auto-magic handling of endorsement with private data collections)
    • Typescript - P4 - Saptha
    • Java - P4
    • Go - P4
  • asset-transfer-secured-agreement (demonstrates auto-magic handling of state-based endorsement)
    • Typescript - P4 - Saptha
    • Java
    • Go
  • off_chain_data (demonstrates block eventing, checkpointing)
    • TypeScript - Mark
    • 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?

@bestbeforetoday bestbeforetoday added the documentation Improvements or additions to documentation label Sep 15, 2021
@bestbeforetoday
Copy link
Member

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
Copy link
Member

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).

@deeptiraom
Copy link

@denyeart Could you please assign the issue to me?

@deeptiraom
Copy link

Currently I am working on the below:

asset-transfer-basic
Java

@deeptiraom
Copy link

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

@denyeart
Copy link
Contributor

denyeart commented Dec 1, 2021

@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

@sapthasurendran
Copy link
Contributor

sapthasurendran commented Dec 15, 2021

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

@deeptiraom
Copy link

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

@denyeart denyeart added this to the 2022Q1 milestone Jan 24, 2022
@denyeart denyeart added the good first issue Good for newcomers label Feb 3, 2022
@denyeart
Copy link
Contributor

denyeart commented Feb 3, 2022

@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 Refactor Developing Applications documentation fabric#2983 / Update App Developer flow fabric#3157 , but keep Commercial Paper sample itself as a good industry example.

What do you think @bestbeforetoday @lehors ?

@lehors
Copy link

lehors commented Feb 3, 2022

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.

@bestbeforetoday
Copy link
Member

bestbeforetoday commented Feb 4, 2022

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.

@denyeart
Copy link
Contributor

denyeart commented Feb 4, 2022

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 ?

@lehors
Copy link

lehors commented Feb 7, 2022

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.

@bestbeforetoday bestbeforetoday modified the milestones: 2022Q1, v1.1 Feb 11, 2022
@bestbeforetoday
Copy link
Member

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?

@HandOfGod94
Copy link

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?

@bestbeforetoday
Copy link
Member

@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

@Sayalikukkar
Copy link

Sayalikukkar commented May 20, 2024

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

@bestbeforetoday
Copy link
Member

bestbeforetoday commented Jun 7, 2024

@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.

@Sayalikukkar
Copy link

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!

@twoGiants
Copy link

Hi @bestbeforetoday! I opened a PR for the asset-transfer-private-data go implementation. This message is a reminder to add a check to the issue above once it's merged.

Best,
Stanislav

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers hacktoberfest https://hacktoberfest.com/ help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

9 participants