-
Notifications
You must be signed in to change notification settings - Fork 286
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
build(api-client): generate go client #3314
build(api-client): generate go client #3314
Conversation
6da167b
to
d731814
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan LGTM, but please wait with merging this until after the openapi spec refaactor that's pending review right now. I don't think this PR should get deeply affected, but just to be on the safe side.
One more thing: The diff is insanely large so I can't review line by line. Could you please confirm that the only additions are the codegen scripts in the package.json files and the generated sources? E.g., there is no other thing in the diff to review that was manually written code?
@petermetz apart from the generated code, these changes were also done
|
@jagpreetsinghsasan Got it, thank you for confirming! LGTM (still) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but please fix the CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plus one for fixing the CI :-)
a089264
to
5f00f4f
Compare
I just had a look at the CI, did some changes, looks like some issue with either the codegen script or the ci script. Looking into the same. Will re-request for review once fixed. Also facing DCI Lint issues because the generated go code has incode comments refering to |
Hi @petermetz But when I run the codegen locally, I dont see those 2 extra files getting created What could be the possible scenario for such a failure (now that we clean the codegen folders as a part of codegen warmup script, such discrepancies shouldn't even exist)? |
@jagpreetsinghsasan I'm not sure either TBH, but there's definitely race conditions in codegen because when I try to run it locally it crashes with the logs pasted below. So, I'd say let's fix the race condition first so that it reliably runs on my machine too and then we can establish an additional data point after that (whether it works on my machine or fails like it does on the CI with those 2 extra files ending up in the diff somehow). Once I'm able to reproduce the issue one way or another then I can try and investigate it more closely. For the race condition itself: Use the lerna-lite ERR! corepack yarn run codegen exited 1 in '@hyperledger/cactus-plugin-persistence-ethereum'
lerna-lite ERR! corepack yarn run codegen stdout:
> @hyperledger/[email protected] generate-sdk
> run-p 'generate-sdk:*'
> @hyperledger/[email protected] generate-sdk:typescript-axios
> openapi-generator-cli generate -i ./src/main/json/openapi.json -g typescript-axios -o ./src/main/typescript/generated/openapi/typescript-axios/ --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore
> @hyperledger/[email protected] generate-sdk:go
> openapi-generator-cli generate -i ./src/main/json/openapi.json -g go -o ./src/main/go/generated/openapi/go-client/ --git-user-id hyperledger --git-repo-id $(echo $npm_package_name | replace @hyperledger/ "" -z)/src/main/go/generated/openapi/go-client --package-name $(echo $npm_package_name | replace @hyperledger/ "" -z) --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore
Download 6.6.0 ...
Download 6.6.0 ...
Downloaded 6.6.0
Downloaded 6.6.0
lerna-lite ERR! corepack yarn run codegen stderr:
Error: An unexpected error occurred while trying to open file /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum/node_modules/@openapitools/openapi-generator-cli/versions/6.6.0.jar
npm ERR! Lifecycle script `generate-sdk:go` failed with error:
npm ERR! Error: command failed
npm ERR! in workspace: @hyperledger/[email protected]
npm ERR! at location: /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum
ERROR: "generate-sdk:go" exited with 1.
npm ERR! Lifecycle script `generate-sdk` failed with error:
npm ERR! Error: command failed
npm ERR! in workspace: @hyperledger/[email protected]
npm ERR! at location: /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum
ERROR: "codegen:openapi" exited with 1.
lerna-lite ERR! corepack yarn run codegen exited 1 in '@hyperledger/cactus-plugin-persistence-ethereum'
lerna-lite WARN complete Waiting for 1 child process to exit. CTRL-C to exit immediately.
file:///home/peter/a/cacti-upstream/node_modules/@lerna-lite/core/node_modules/execa/lib/error.js:60
error = new Error(message);
^
Error: Command failed with exit code 1: corepack yarn run codegen
Error: An unexpected error occurred while trying to open file /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum/node_modules/@openapitools/openapi-generator-cli/versions/6.6.0.jar
npm ERR! Lifecycle script `generate-sdk:go` failed with error:
npm ERR! Error: command failed
npm ERR! in workspace: @hyperledger/[email protected]
npm ERR! at location: /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum
ERROR: "generate-sdk:go" exited with 1.
npm ERR! Lifecycle script `generate-sdk` failed with error:
npm ERR! Error: command failed
npm ERR! in workspace: @hyperledger/[email protected]
npm ERR! at location: /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum
ERROR: "codegen:openapi" exited with 1.
> @hyperledger/[email protected] generate-sdk
> run-p 'generate-sdk:*'
> @hyperledger/[email protected] generate-sdk:typescript-axios
> openapi-generator-cli generate -i ./src/main/json/openapi.json -g typescript-axios -o ./src/main/typescript/generated/openapi/typescript-axios/ --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore
> @hyperledger/[email protected] generate-sdk:go
> openapi-generator-cli generate -i ./src/main/json/openapi.json -g go -o ./src/main/go/generated/openapi/go-client/ --git-user-id hyperledger --git-repo-id $(echo $npm_package_name | replace @hyperledger/ "" -z)/src/main/go/generated/openapi/go-client --package-name $(echo $npm_package_name | replace @hyperledger/ "" -z) --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore
Download 6.6.0 ...
Download 6.6.0 ...
Downloaded 6.6.0
Downloaded 6.6.0
at makeError (file:///home/peter/a/cacti-upstream/node_modules/@lerna-lite/core/node_modules/execa/lib/error.js:60:11)
at handlePromise (file:///home/peter/a/cacti-upstream/node_modules/@lerna-lite/core/node_modules/execa/index.js:124:26)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async file:///home/peter/a/cacti-upstream/node_modules/p-queue/dist/index.js:187:36 {
shortMessage: 'Command failed with exit code 1: corepack yarn run codegen',
command: 'corepack yarn run codegen',
escapedCommand: 'corepack yarn run codegen',
exitCode: 1,
signal: undefined,
signalDescription: undefined,
stdout: '\n' +
'> @hyperledger/[email protected] generate-sdk\n' +
"> run-p 'generate-sdk:*'\n" +
'\n' +
'\n' +
'> @hyperledger/[email protected] generate-sdk:typescript-axios\n' +
'> openapi-generator-cli generate -i ./src/main/json/openapi.json -g typescript-axios -o ./src/main/typescript/generated/openapi/typescript-axios/ --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore\n' +
'\n' +
'\n' +
'> @hyperledger/[email protected] generate-sdk:go\n' +
'> openapi-generator-cli generate -i ./src/main/json/openapi.json -g go -o ./src/main/go/generated/openapi/go-client/ --git-user-id hyperledger --git-repo-id $(echo $npm_package_name | replace @hyperledger/ "" -z)/src/main/go/generated/openapi/go-client --package-name $(echo $npm_package_name | replace @hyperledger/ "" -z) --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore\n' +
'\n' +
'Download 6.6.0 ...\n' +
'Download 6.6.0 ...\n' +
'Downloaded 6.6.0\n' +
'Downloaded 6.6.0',
stderr: 'Error: An unexpected error occurred while trying to open file /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum/node_modules/@openapitools/openapi-generator-cli/versions/6.6.0.jar\n' +
'npm ERR! Lifecycle script `generate-sdk:go` failed with error: \n' +
'npm ERR! Error: command failed \n' +
'npm ERR! in workspace: @hyperledger/[email protected] \n' +
'npm ERR! at location: /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum \n' +
'ERROR: "generate-sdk:go" exited with 1.\n' +
'npm ERR! Lifecycle script `generate-sdk` failed with error: \n' +
'npm ERR! Error: command failed \n' +
'npm ERR! in workspace: @hyperledger/[email protected] \n' +
'npm ERR! at location: /home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum \n' +
'ERROR: "codegen:openapi" exited with 1.',
cwd: '/home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum',
failed: true,
timedOut: false,
isCanceled: false,
killed: false,
pkg: Package {
_id: '',
isBumpOnlyVersion: false,
licensePath: '',
localDependencies: Map(0) {},
name: '@hyperledger/cactus-plugin-persistence-ethereum',
[Symbol(location)]: '/home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum',
[Symbol(resolved)]: Result {
type: 'directory',
registry: undefined,
where: '/home/peter/a/cacti-upstream',
raw: '@hyperledger/cactus-plugin-persistence-ethereum@file:packages/cactus-plugin-persistence-ethereum',
name: '@hyperledger/cactus-plugin-persistence-ethereum',
escapedName: '@hyperledger%2fcactus-plugin-persistence-ethereum',
scope: '@hyperledger',
rawSpec: 'file:packages/cactus-plugin-persistence-ethereum',
saveSpec: 'file:packages/cactus-plugin-persistence-ethereum',
fetchSpec: '/home/peter/a/cacti-upstream/packages/cactus-plugin-persistence-ethereum',
gitRange: undefined,
gitCommittish: undefined,
gitSubdir: undefined,
hosted: undefined
},
[Symbol(rootPath)]: '/home/peter/a/cacti-upstream',
[Symbol(scripts)]: {
build: 'npm run build-ts && npm run build:dev:backend:postbuild',
'build-ts': 'tsc',
'build:dev:backend:postbuild': 'npm run copy-sql && npm run copy-yarn-lock',
codegen: "run-p 'codegen:*'",
'codegen:openapi': 'npm run generate-sdk',
'copy-sql': 'mkdir -p ./dist/lib/main/ && cp -Rfp ./src/main/sql ./dist/lib/main/',
'copy-yarn-lock': 'mkdir -p ./dist/lib/ && cp -rfp ../../yarn.lock ./dist/yarn.lock',
'generate-sdk': "run-p 'generate-sdk:*'",
'generate-sdk:typescript-axios': 'openapi-generator-cli generate -i ./src/main/json/openapi.json -g typescript-axios -o ./src/main/typescript/generated/openapi/typescript-axios/ --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore',
'generate-sdk:go': 'openapi-generator-cli generate -i ./src/main/json/openapi.json -g go -o ./src/main/go/generated/openapi/go-client/ --git-user-id hyperledger --git-repo-id $(echo $npm_package_name | replace @hyperledger/ "" -z)/src/main/go/generated/openapi/go-client --package-name $(echo $npm_package_name | replace @hyperledger/ "" -z) --reserved-words-mappings protected=protected --ignore-file-override ../../openapi-generator-ignore'
}
}
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, concur with Peter and Michal on fixing the CI. Any specific reason why we are adding go SDKs to all plugins? (Sorry for my absence recently but I've been out due to illness). I was going to add a Go SDK for the SATP plugin in any case. TS + Go seem like very reasonable options.
I am not sure why is it failing on your machine @petermetz , but even with the |
@jagpreetsinghsasan @petermetz Worked on my machine as well, the CI is still failing though ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the failing checks are required I'll give apporval not to slow down the merge once it's fixed :)
@RafaelAPB My own take: Adding the client implementations it to all the plugins increases the chance that people will find the project useful, e.g. we don't know which plugin someone would be starting off with but whichever it is they'd be happy to have a go client (assuming they'd want a go client). The downside is that the codegen script is getting longer and longer so this change also puts it in the spotlight that we should have the script altered in a way that it either
|
c55bbcc
to
861e5f5
Compare
I do agree with that. Do you have any solution in mind? I thought about publishing the packages in third-party repos (such as npm or others but we risk some decentralization. We could publish them in separate repos that have a job such that it scraps updates to the open API files, generates, and publishes the SDKs. |
@RafaelAPB I laid out my ideas here in more detail: https://github.com/hyperledger/cacti/issues/3403 I'm also not opposed to creating more repositories as long as other people volunteer to be the principal maintainer on those repositories (the overhead of managing one repo is more than enough for me so I can't volunteer to take care of the admin chores of any more repositories unfortunately) |
@jagpreetsinghsasan Attaching the codegen failure log: looks like more of the same where the yarn |
@petermetz , this is actually a good idea. Would also reduce the chores on the main repo. We had similar issues maintaining all possible examples in Bevel, so we divided the project into 2 parts, one having the core code and other holding the examples |
I will update that asap and re-request for review |
@petermetz will that adding to |
@jagpreetsinghsasan I'd say yes. That way it's always pointing to the same installation of the generator cli package and therefore re-uses the pre-downloaded .jar file that we get in the warmup codegen script in the root package.json |
73c7a44
to
68674ef
Compare
@petermetz I am not sure why the tools:validate-bundle-names is failing ( |
@jagpreetsinghsasan You're adding the webpack scripts that I've removed from this package in PR merged 4 days ago, I guess that is triggering this error. Remove the webpack scripts and it should pass I think :) |
Oh right @outSH Update: Fixed |
68674ef
to
7368fa5
Compare
@petermetz I have updated the code. The codegen seems fine now (it never failed for me, so can you verify on your end?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan LGMT just please fix the DCI lint check.
I think you can ignore **/src/main/go/generated/openapi/go-client/client.go
in .dcilintignore and then it should be OK. Just make sure to include an explanation that the generated sources that we don't control have the failing text in them.
5e60e80
to
5f44c67
Compare
Primary Changes ---------------- 1. Updated package.json file for packages to include the new codegen script 2. Added a new dep, replace for string manupulation in package.json Fixes hyperledger-cacti#393 Signed-off-by: jagpreetsinghsasan <[email protected]>
Commit to be reviewed
build(api-client): generate go client
Fixes #393
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.