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

ci(codegen)!: fix the yarn codegen job in ci.yaml #2633

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

petermetz
Copy link
Contributor

@petermetz petermetz commented Aug 23, 2023

BREAKING CHANGES:

  1. Fabric connector endpoints are using base64 encoding instead of JSON
    when needing to transfer otherwise binary data (UInt8Array)
  2. Same as the above for the Iroha 1 and Iroha 2 connectors.

Primary changes:

  1. Removed the "identifier": "Apache-2.0", section from all OpenAPI
    specification documents (./**/openapi.json) project-wide because the
    OpenAPI generator is not yet compatible with it and because of that the
    new property just makes it crash with a false-negative validation error.
  2. Downgraded all OpenAPI specification documents version from v3.1.0 to
    v3.0.3 (which is how it used to be) because the generator is not yet
    compatible with it and it had caused types to be mangled.
  3. Eliminated incorrect usage of the "format": "binary" declarations in
    the OpenAPI specifications where the format was declared as binary but
    the data was still traveling as JSON strings despite it (divergence
    between our spec .json files and the implementation under the hood)
  4. Updated the tests and their related endpoints to use base64 encoding
    instead of JSON serializing UInt8Array instances: It is more efficient
    this way and also has the benefit of honoring the specification. The
    reason why it's more efficient is because JSON serializing a typed UInt8Array
    results in an object literal that maps every single byte to a property
    of said object, roughly tripling the size of the network transfer. It
    probably also is slower to serialize/deserialize but I haven't actually
    measured this.
  5. All of the packages are now using the OpenAPI generator v6.6.0 instead
    of some being stuck on v5.2.1. This might cause build issues in kotlin
    API clients but because we don't have a freeze on breaking changes right
    now (leading up to v2.0.0) it is the right time to make drastic changes
    like this.

Fixes #2618
Fixes https://github.com/hyperledger/cacti/issues/2299

Signed-off-by: Peter Somogyvari [email protected]

@petermetz
Copy link
Contributor Author

All: Apologies for the huge diff, changing the generator configuration unfortunately has this bad habit of generating huge diffs.
I also had to break it up to two separate commits so that the changes that are not (strictly) related to the generator update are in a separate commit for easier reading.

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
This also fixes #2299

jagpreetsinghsasan
jagpreetsinghsasan approved these changes Aug 24, 2023
Copy link
Contributor

@izuru0 izuru0 left a comment

Choose a reason for hiding this comment

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

LGTM

1. If we run the codegen script and it generates a diff (e.g. changes files
that are under version control) that means that we have to send it back
to the person who is making the pull request and ask them to include
those changes in their commit so that we are adhering to the idea of
reproducible builds as much as possible.
2. These (env var) parameters of ./tools/ci.sh are now true by default:
  - `TOOLS_VALIDATE_BUNDLE_NAMES_DISABLED`
  - `CUSTOM_CHECKS_DISABLED`
  - `FULL_BUILD_DISABLED`
3. Refactored the custom-check that verifies the Open API specs' license parameters
(`tools/custom-checks/check-open-api-json-specs.ts`)
so that it does not fail for missing "identifier" properties UNLESS the spec's
version is 3.1.0 or higher. This was necessary because we cannot yet migrate the
specs to v3.1.0 due to the generator not supporting it properly (it ends up
mangling all of the types in the generated code from their actual type to `any`)
4. Additional logging was also added to the script mentioned in 3) to make it
easier in the future to debug what is the problem.
5. Added these environment variables to control the behavior of the
`tools/custom-checks/check-open-api-json-specs.ts` script:
  - `CACTI_CUSTOM_CHECKS_REQUIRED_OPENAPI_SPEC_VERSION`
  - `CACTI_CUSTOM_CHECKS_REQUIRED_OPENAPI_LICENSE_SPDX`
  - `CACTI_CUSTOM_CHECKS_REQUIRED_OPENAPI_LICENSE_URL`

Related to hyperledger-cacti#2618

Signed-off-by: Peter Somogyvari <[email protected]>
BREAKING CHANGES:
1. Fabric connector endpoints are using base64 encoding instead of JSON
when needing to transfer otherwise binary data (UInt8Array)
2. Same as the above for the Iroha 1 and Iroha 2 connectors.

Primary changes:
-----------------
1. Removed the `"identifier": "Apache-2.0",` section from all OpenAPI
specification documents (./**/openapi.json) project-wide because the
OpenAPI generator is not yet compatible with it and because of that the
new property just makes it crash with a false-negative validation error.
2. Downgraded all OpenAPI specification documents version from `v3.1.0` to
`v3.0.3` (which is how it used to be) because the generator is not yet
compatible with it and it had caused types to be mangled.
3. Eliminated incorrect usage of the "format": "binary" declarations in
the OpenAPI specifications where the format was declared as binary but
the data was still traveling as JSON strings despite it (divergence
between our spec .json files and the implementation under the hood)
4. Updated the tests and their related endpoints to use base64 encoding
instead of JSON serializing UInt8Array instances: It is more efficient
this way and also has the benefit of honoring the specification. The
reason why it's more efficient is because JSON serializing a typed UInt8Array
results in an object literal that maps every single byte to a property
of said object, roughly tripling the size of the network transfer. It
probably also is slower to serialize/deserialize but I haven't actually
measured this.
5. All of the packages are now using the OpenAPI generator v6.6.0 instead
of some being stuck on v5.2.1. This might cause build issues in kotlin
API clients but because we don't have a freeze on breaking changes right
now (leading up to v2.0.0) it is the right time to make drastic changes
like this.

Fixes hyperledger-cacti#2618

Signed-off-by: Peter Somogyvari <[email protected]>
@petermetz petermetz merged commit e8033dd into hyperledger-cacti:main Aug 28, 2023
122 of 133 checks passed
@petermetz petermetz deleted the petermetz/issue2618 branch September 5, 2023 16:50
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.

ci(codegen): fix the yarn codegen job in ci.yaml feat(openapi): upgrade to 6.3.0 - phase 2
3 participants