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

[KOGITO-5719] Transform Kogito Add-Ons to Quarkus Extensions #1554

Merged
merged 15 commits into from
Sep 20, 2021

Conversation

ricardozanini
Copy link
Member

@ricardozanini ricardozanini commented Aug 24, 2021

Signed-off-by: Ricardo Zanini [email protected]

See https://issues.redhat.com/browse/KOGITO-5719

Linked PRs:

In this PR, we introduce a "small" change to our Quarkus based add-ons. Now, everyone is also a Quarkus Extension. This is important to unlock the new Add-On Codegen API since we will leverage the ability of Quarkus to inject the *-deployment modules on-demand on end-users projects during code generation. The idea is to add all the codegen related code for add-ons on the *-deployment modules backed by this new shared API. This way, SB can leverage this mechanism as well.

As a plus, we now can appear at code.quarkus.io as an option in the Quarkus project creation. Thus, users will be able to select "kogito-rules" and "kogito-monitoring-prometheus" from a list of available extensions, for example.

All our internal builder scripts on the cloud can also leverage this change to add add-ons on-demand simply using the Quarkus maven plugin.

This change is a step forward in leveraging the Quarkus platform.

I've also deprecated the "generic" add-ons. I kept only the "human-task-prediction" since we need to know what to do with this add-on. Seems unused, maybe a good candidate for deprecation.

I haven't created/renamed the generic persistence add-ons for SB in this PR because we have a JIRA to deal with it in the short term. For example KOGITO-5031. @cristianonicolai, do you mind linking the others in this JIRA?

The Quarkus persistence add-ons are a simple "redirect" to the actual common ones. It's not in the scope of this PR to refactor them in the "Quarkus way". @cristianonicolai, if you don't mind we can sync up these next changes in the Persistence sync.

That's cool:

2021-08-27 13:23:21,022 INFO  [io.quarkus] (main) Installed features: [agroal, cdi, jdbc-postgresql, kogito-addon-persistence-jdbc-extension, kogito-decisions, kogito-predictions, kogito-processes, kogito-rules, narayana-jta, resteasy, resteasy-jackson, servlet, smallrye-context-propagation, smallrye-health, smallrye-openapi, swagger-ui]

😎

Many thanks for submitting your Pull Request ❤️!

Please make sure that your PR meets the following requirements:

  • You have read the contributors guide
  • Your code is properly formatted according to this configuration
  • Pull Request title is properly formatted: KOGITO-XYZ Subject
  • Pull Request title contains the target branch if not targeting main: [0.9.x] KOGITO-XYZ Subject
  • Pull Request contains link to the JIRA issue
  • Pull Request contains link to any dependent or related Pull Request
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket
How to retest this PR or trigger a specific build:
  • Run all builds
    Please add comment: Jenkins retest this

  • Run (or rerun) specific test(s)
    Please add comment: Jenkins (re)run [runtimes|optaplanner|apps|examples] tests

  • Quarkus LTS checks
    Please add comment: Jenkins run LTS

  • Run (or rerun) LTS specific test(s)
    Please add comment: Jenkins (re)run [runtimes|optaplanner|apps|examples] LTS

  • Native checks
    Please add comment: Jenkins run native

  • Run (or rerun) native specific test(s)
    Please add comment: Jenkins (re)run [runtimes|optaplanner|apps|examples] native

  • Full Kogito testing (with cloud images and operator BDD testing)
    Please add comment: Jenkins run BDD
    This check should be used only if a big change is done as it takes time to run, need resources and one full BDD tests check can be done at a time ...

@apache apache deleted a comment from kie-ci Aug 24, 2021
@apache apache deleted a comment from kie-ci Aug 24, 2021
@apache apache deleted a comment from kie-ci Aug 24, 2021
@apache apache deleted a comment from kie-ci Aug 24, 2021
@apache apache deleted a comment from kie-ci Aug 24, 2021
@apache apache deleted a comment from kie-ci Aug 24, 2021
@kie-ci
Copy link
Contributor

kie-ci commented Aug 26, 2021

The (build) Examples check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Aug 26, 2021

The (build) Apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Aug 26, 2021

The (build) Runtimes check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Aug 27, 2021

The (build) Apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Aug 27, 2021

The (build) Apps check has failed. Please check the logs.

@ricardozanini ricardozanini marked this pull request as ready for review August 27, 2021 19:05
@ricardozanini ricardozanini changed the title [KOGITO-5719] Transforn Kogito Add-Ons to Quarkus Extensions [KOGITO-5719] Transform Kogito Add-Ons to Quarkus Extensions Aug 27, 2021
@ricardozanini
Copy link
Member Author

cc @spolti, can you assess the impact on the images? I can't think of one at the top of my head right now. I believe it won't be any.

@kie-ci
Copy link
Contributor

kie-ci commented Aug 27, 2021

The (build) Runtimes check has failed. Please check the logs.

Copy link
Contributor

@fjtirado fjtirado left a comment

Choose a reason for hiding this comment

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

Great work Ricardo.
Once this is merged, I would like to restructure dependencies between decission and messaging addons.
Right now decission events depends on messaging add-on. With a couple of changes (moving the two classes in messaging runtime addon to messaging common addon) we can make decissions depend on common-messaging addon, so we can avoid code generation for messaging when only decission addon is added to dependency.
Final situation will be messaging runtime become empty (it will include classes in common though, which are shared with decissions and knative addons) and will just consist on the code generation part for smallrye.

@ricardozanini
Copy link
Member Author

Discussions with Quarkus team about having all these extensions on quarkus-platform: https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/Kogito.20Add-Ons.20as.20Extensions.20.28or.20Extensions.20of.20Extensions.29

Copy link
Contributor

@cristianonicolai cristianonicolai left a comment

Choose a reason for hiding this comment

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

@ricardozanini this is great, nice work! Just added some comments inline.

Copy link
Contributor

@danielezonca danielezonca left a comment

Choose a reason for hiding this comment

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

I think the PR is ready, just minor comments

@ricardozanini
Copy link
Member Author

@danielezonca @cristianonicolai, I believe I covered all the minor details. It can be merged at any time.

Copy link
Contributor

@danielezonca danielezonca left a comment

Choose a reason for hiding this comment

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

Great work! 👍

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 7 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 31 Code Smells

52.8% 52.8% Coverage
0.0% 0.0% Duplication

Copy link
Member

@MarianMacik MarianMacik left a comment

Choose a reason for hiding this comment

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

Looks good, just 2 suggestions/questions.

@ricardozanini ricardozanini merged commit 1ecd034 into apache:main Sep 20, 2021
@ricardozanini ricardozanini deleted the kogito-5719 branch September 20, 2021 14:17
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.

7 participants