search

hyperledger / fabric-gateway

Go, Node and Java client API for Hyperledger Fabric v2.4+
https://hyperledger.github.io/fabric-gateway/
Apache License 2.0
158 stars 92 forks source link

Migrate fabric-samples to use Gateway SDKs #190

Open jt-nti opened 3 years ago

jt-nti commented 3 years ago

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

Lower priority

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

bestbeforetoday commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@denyeart Could you please assign the issue to me?

deeptiraom commented 3 years ago

Currently I am working on the below:

asset-transfer-basic Java

deeptiraom commented 3 years ago

I have done the following so far:

denyeart commented 3 years ago

@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 commented 2 years ago

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

deeptiraom commented 2 years ago

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

denyeart commented 2 years ago

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

What do you think @bestbeforetoday @lehors ?

lehors commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 6 months ago

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

bestbeforetoday commented 5 months ago

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

Let me know if you have any questions.

Sayalikukkar commented 5 months ago

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 commented 1 month ago

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