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.

Sign up for GitHub

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

Jump to bottom

feat(fabric-test-ledger): add support to enrolling users in different Orgs #2249

Merged
merged 1 commit into from
Aug 14, 2023
Merged

feat(fabric-test-ledger): add support to enrolling users in different Orgs #2249

merged 1 commit into from
Aug 14, 2023
+135 −57

Conversation

AndreAugusto11
Copy link
Contributor

@AndreAugusto11 AndreAugusto11 commented Jan 8, 2023 •
edited
Loading

Created new methods to avoid breaking changes in the API exported.

New methods created:

  • capitalizedMspIdOfOrg
  • enrollAdminV2
  • enrollUserV2
  • createCaClientV2

closes #2248

Signed-off-by: André Augusto [email protected]

@AndreAugusto11 AndreAugusto11 mentioned this pull request Jan 8, 2023
Merged
jagpreetsinghsasan
jagpreetsinghsasan approved these changes Jan 9, 2023
View reviewed changes
Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

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

LGTM

petermetz
petermetz approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@AndreAugusto11 Thanks, much easier to review this in smaller chunks!

@jagpreetsinghsasan
Copy link
Contributor

I have one question here for @petermetz
When does api version (like in this case v2) in our code, needs to be v2? Should we not tie the api version to our cacti version or something similar sort of mapping, so that its the same everywhere? Otherwise tracking where-all its v1 and where-all its v2, and why the upgrade was done will be troublesome to track (Specially for the devs who are using Cacti and want to upgrade to a newer version of code)
The issue #2248 is more of a bug than an enhancement (in my opinion), and to fix the bug, we can still use the v1 version with the updated code.

petermetz
petermetz requested changes Jan 31, 2023
View reviewed changes
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@AndreAugusto11 This still LGTM, but we have a failing test so please fix that before we merge!

Logs:
2023-01-31_155022_cacti_ci_crash_fabric_connector.log

@jagpreetsinghsasan
Copy link
Contributor

@AndreAugusto11 any updates on this? (Asking as #2185 is dependent on this)

@AndreAugusto11 @petermetz
feat(fabric-test-ledger): add support to enrolling users in different…
0a3321c 
… Orgs

Created new methods to avoid breaking changes in the API exported
New methods created:
* getConnectionProfileOrgX
* enrollAdminV2
* enrollUserV2
* createCaClientV2

closes hyperledger-cacti#2248

Co-authored-by: Peter Somogyvari <[email protected]>

Signed-off-by: André Augusto <[email protected]>
Signed-off-by: Peter Somogyvari <[email protected]>
petermetz
petermetz approved these changes Aug 14, 2023
View reviewed changes
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@AndreAugusto11 I added the fixes, LGTM

@petermetz petermetz enabled auto-merge (rebase) August 14, 2023 06:42
@petermetz petermetz merged commit b910681 into hyperledger-cacti:main Aug 14, 2023
108 of 117 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petermetz petermetz petermetz approved these changes

@jagpreetsinghsasan jagpreetsinghsasan jagpreetsinghsasan approved these changes

@takeutak takeutak takeutak approved these changes

@izuru0 izuru0 Awaiting requested review from izuru0 izuru0 is a code owner

@sandeepnRES sandeepnRES Awaiting requested review from sandeepnRES sandeepnRES is a code owner

@VRamakrishna VRamakrishna Awaiting requested review from VRamakrishna VRamakrishna is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(fabric-test-ledger): extend implementation to enroll users in different Orgs
4 participants
@AndreAugusto11 @jagpreetsinghsasan @petermetz @takeutak