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

kie-issues#2601: [kn-plugin-workflow] Minify the openAPI spec files to trim operations only used by the workflows in the current project #2711

Merged
merged 10 commits into from
Nov 13, 2024
Merged

kie-issues#2601: [kn-plugin-workflow] Minify the openAPI spec files to trim operations only used by the workflows in the current project #2711

merged 10 commits into from
Nov 13, 2024

Conversation

treblereel
Copy link
Contributor

@treblereel treblereel commented Oct 27, 2024 •
edited by ricardozanini
Loading

Fix #2601

tests are missed atm, I'll add it in a couple of days

ricardozanini
ricardozanini reviewed Oct 29, 2024
View reviewed changes
Copy link
Member

@ricardozanini ricardozanini left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You can take a few openapi specs as an example for tests from here:
https://github.com/quarkiverse/quarkus-openapi-generator/tree/main/client/integration-tests

Things that should be included in the test cases:

  • Full openapi spec
  • Minified spec
  • Workflow referencing the spec
  • Adding another openapi operation to the workflow and recheck if the new operation has been added to the minified version.

packages/kn-plugin-workflow/pkg/common/openapi_minifier.go Outdated Show resolved Hide resolved
packages/kn-plugin-workflow/pkg/command/deploy_undeploy_common.go Outdated Show resolved Hide resolved
packages/kn-plugin-workflow/pkg/common/openapi_minifier.go Outdated Show resolved Hide resolved
packages/kn-plugin-workflow/pkg/common/openapi_minifier.go Outdated Show resolved Hide resolved
@treblereel
Copy link
Contributor Author

@ricardozanini I have added --minify option to the deploy, it allows us to skip minification if needed
By default it's true

ricardozanini
ricardozanini reviewed Nov 1, 2024
View reviewed changes
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

Well done! I left two minor suggestions.

packages/kn-plugin-workflow/pkg/specs/openapi_minifier.go Outdated Show resolved Hide resolved
packages/kn-plugin-workflow/pkg/specs/openapi_minifier.go Outdated Show resolved Hide resolved
@ricardozanini
Copy link
Member

@domhanak can you please verify this?

@domhanak
Copy link
Contributor

domhanak commented Nov 6, 2024

@domhanak can you please verify this?

Yup, working on it, investigating some issue with compilation after the minifications, Will let you know.

@domhanak
Copy link
Contributor

domhanak commented Nov 11, 2024 •
edited
Loading

Documentation of the command

  • kn workflow specs help - ✔️
  • kn workflow specs minify -> this does nothing, and when I do kn workflow specs minify help the result is the same. I think we can better explain the command to the user, in a sense he understands openapi needs to be added as argument to start the minification. For example adding more targeted HELP description here 🚧

Functional

  • kn workflow specs minify openapi
    • Input results in output -> Is that correct? I can see a ConfigMap 01-hello-resources-specs.yaml when deploying but the workflow is failing with compilation errors.
      👀 investigating why 👀
      Cause: Typo in function definition of the workflow, once fixed workflow properly deploys without errors. ✔️
    • components are part of the minified file, is that expected? ❓
    • file size seems to be larger after minification, according to the message :white_check_mark: Minified file specs/greetingAPI.min.yaml created with 7572 bytes (original size: 6212 bytes)

More information about testing to follow, I will update the comment with latest info.
Double checking the scenarios with subflows.

@ricardozanini ricardozanini merged commit 8531160 into apache:main Nov 13, 2024
14 checks passed
rgdoliveira pushed a commit to rgdoliveira/kie-tools that referenced this pull request Nov 13, 2024
@treblereel @rgdoliveira
kie-issues#2601: [kn-plugin-workflow] Minify the openAPI spec files t…
eca0047 
…o trim operations only used by the workflows in the current project (apache#2711)

Co-authored-by: Dmitrii Tikhomirov <treblereel@Dmitriis-MacBook-Pro.local>
rgdoliveira added a commit to kiegroup/kie-tools that referenced this pull request Nov 13, 2024
@rgdoliveira @treblereel
kie-issues#2601: [kn-plugin-workflow] Minify the openAPI spec files t…
0120cb8 
…o trim operations only used by the workflows in the current project (apache#2711) (#62)

Co-authored-by: Dmitrii Tikhomirov <treblereel@users.noreply.github.com>
Co-authored-by: Dmitrii Tikhomirov <treblereel@Dmitriis-MacBook-Pro.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domhanak domhanak domhanak approved these changes

@ricardozanini ricardozanini ricardozanini approved these changes

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

@wmedvede wmedvede Awaiting requested review from wmedvede

Assignees
No one assigned
Labels
area:sonataflow
Projects
None yet
Milestone
No milestone
4 participants
@treblereel @ricardozanini @domhanak @tiagobento