Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
kie-issues#2789: [sonataflow-operator] Implement DB Migrator Changes into SonataFlow Operator #2790
base: main
Are you sure you want to change the base?
kie-issues#2789: [sonataflow-operator] Implement DB Migrator Changes into SonataFlow Operator #2790
Changes from 10 commits
ab197e9
450006b
1d57981
00e2a1d
0a0bf99
32a4588
6042723
0f71572
8bc3c37
d3a8809
c71af46
25475df
f0687a7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
do not forget to replace with apache image
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.
Agreed as soon as the image PR is approved and image is available, we will do the same.
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.
I think the dbMigrationStrategy must be added to the DI and JS in this file too.
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.
@wmedvede Where are DI and JS in this file? Please advise.
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.
@rhkp your observation is good, and mine too 🤔
Rethinking here, It looks like we have a sort of outdated (and not necessarily outdated) specification of the CustomResourceDefinitions in this file... i.e., not all the fields were always added to the respective CRDs.
Some fields were added, and some other don't.
For example:
we have things like
path: services.jobService.podTemplate
but we don't have things likeservices.jobService.persistence
Which makes me think that not all the fields for the different CRDs owned by this operator where added to this file along the time. Question is, do we need them all added there? I think that not, see below.
Also, by looking at here: https://olm.operatorframework.io/docs/concepts/crds/clusterserviceversion I think the full specification of each CRD in that list is not needed (that information is not used for creating the respective CRDs), instead we could just only declare file the list of owned resources by this operator....
@ricardozanini is my reasoning correct?