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

feat(openapi): upgrade to 6.3.0 phase1 #2333

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

jagpreetsinghsasan
Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan commented Mar 22, 2023

Commit to be reviewed


feat(openapi): upgrade to 6.3.0 phase1

Primary Changes
---------------
1. Updated openapitoos.json for various plugins to use 6.3.0
version of openapi generator
2. Relevant code for each plugin was generated

openapitools.json is updated for the following plugins to
incorporate 1)
------------------------------------------------------------
a. extensions/cactus-plugin-htlc-coordinator-besu
b. packages/cactus-plugin-htlc-eth-besu
c. packages/cactus-plugin-htlc-eth-besu-erc20
d. packages/cactus-plugin-ledger-connector-besu
e. packages/cactus-plugin-ledger-connector-quorum
f. packages/cactus-cmd-api-server
g. packages/cactus-core-api
h. packages/cactus-plugin-consortium-manual
i. packages/cactus-plugin-keychain-aws-sm
j. packages/cactus-plugin-keychain-azure-kv
k. packages/cactus-plugin-keychain-google-sm
l. packages/cactus-plugin-keychain-memory
m. packages/cactus-plugin-keychain-memory-wasm
n. packages/cactus-plugin-keychain-vault
o. packages/cactus-plugin-ledger-connector-ubiquity
p. packages/cactus-plugin-ledger-connector-xdai packages/cactus-plugin-odap-hermes
q. examples/cactus-example-carbon-accounting-business-logic-plugin

Fixes #2298
Depends on #2272

Copy link
Member

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

@jagpreetsinghsasan LGTM, thank you for breaking this up into more digestible chunks!
I did a rebase on it, hopefully the CI will start passing, it seemed like the build failed due to some network issue on the server side (couldn't fetch the jar of the openapi generator).
I tested it on my local machine and it worked fine.

@jagpreetsinghsasan
Copy link
Contributor Author

@jagpreetsinghsasan LGTM, thank you for breaking this up into more digestible chunks! I did a rebase on it, hopefully the CI will start passing, it seemed like the build failed due to some network issue on the server side (couldn't fetch the jar of the openapi generator). I tested it on my local machine and it worked fine.

@petermetz we will need this PR to be reviewed and merged first: #2272 (once 2272 gets merged, I will run a quick test again on this PR)

@petermetz
Copy link
Member

@petermetz we will need this PR to be reviewed and merged first: #2272 (once 2272 gets merged, I will run a quick test again on this PR)

@jagpreetsinghsasan Whaaat. Sorry I must've messed something up with the issue creation. I meant to define the tasks in a way that phase 2 depends on phase 1 being done first not the other way around. 😅
Anyway, if that is how it is now with dependencies then please make sure to mark this as a dependent of the phase 2 PR that you linked to otherwise I might accidentally merge this first (the robot helps a lot by blocking the merge when there's unresolved PR dependencies)

@petermetz petermetz self-assigned this Mar 29, 2023
@jagpreetsinghsasan
Copy link
Contributor Author

@petermetz we will need this PR to be reviewed and merged first: #2272 (once 2272 gets merged, I will run a quick test again on this PR)

@jagpreetsinghsasan Whaaat. Sorry I must've messed something up with the issue creation. I meant to define the tasks in a way that phase 2 depends on phase 1 being done first not the other way around. sweat_smile Anyway, if that is how it is now with dependencies then please make sure to mark this as a dependent of the phase 2 PR that you linked to otherwise I might accidentally merge this first (the robot helps a lot by blocking the merge when there's unresolved PR dependencies)

@petermetz , no no, you havent created the issues in the wrong order.

The PR: #2272 is the phase 2 of generating kotlin clients
If that gets merged, we will have kotlin clients (with the 5.x version of openapi-generator)

Once that gets merged, I need to re-test this PR again (because now its 6.x version of openapi-generator in this PR, and even that might have some conflicts with the kotlin clients). This is phase 1 of 6.x version upgrade.

@github-actions
Copy link

This PR/issue depends on:

Copy link
Member

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

@jagpreetsinghsasan The build started breaking on the CI after rebasing, please take a look!
2023-04-18T04:58:36-ci-crash.log

Copy link
Member

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

@jagpreetsinghsasan LGTM

FYI: I resolved the merge conflicts and rebased this onto upstream/main

    Primary Changes
    ---------------
    1. Updated openapitoos.json for various plugins to use 6.3.0
    version of openapi generator
    2. Relevant code for each plugin was generated

    openapitools.json is updated for the following plugins to
    incorporate 1)
    ------------------------------------------------------------
    a. extensions/cactus-plugin-htlc-coordinator-besu
    b. packages/cactus-plugin-htlc-eth-besu
    c. packages/cactus-plugin-htlc-eth-besu-erc20
    d. packages/cactus-plugin-ledger-connector-besu
    e. packages/cactus-plugin-ledger-connector-quorum
    f. packages/cactus-cmd-api-server
    g. packages/cactus-core-api
    h. packages/cactus-plugin-consortium-manual
    i. packages/cactus-plugin-keychain-aws-sm
    j. packages/cactus-plugin-keychain-azure-kv
    k. packages/cactus-plugin-keychain-google-sm
    l. packages/cactus-plugin-keychain-memory
    m. packages/cactus-plugin-keychain-memory-wasm
    n. packages/cactus-plugin-keychain-vault
    o. packages/cactus-plugin-ledger-connector-ubiquity
    p. packages/cactus-plugin-ledger-connector-xdai packages/cactus-plugin-odap-hermes
    q. examples/cactus-example-carbon-accounting-business-logic-plugin

Fixes hyperledger#2298

Co-authored-by: Peter Somogyvari <peter.somogyvari@accenture.com>

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Signed-off-by: jagpreetsinghsasan <jagpreet.singh.sasan@accenture.com>
@petermetz petermetz merged commit a094614 into hyperledger:main Jun 13, 2023
108 of 117 checks passed
@petermetz petermetz deleted the feature-2298 branch June 13, 2023 18:46
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.

feat(openapi): upgrade to 6.3.0 - phase 1
3 participants