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

[KOGITO-7754] create Knative eventing resources #350

Merged
merged 1 commit into from
Feb 28, 2024
Merged

[KOGITO-7754] create Knative eventing resources #350

merged 1 commit into from
Feb 28, 2024

Conversation

RichardW98
Copy link
Contributor

@RichardW98 RichardW98 commented Jan 15, 2024 •
edited
Loading

create Knative resources:

  1. create sinkbinding and trigger if spec.flow.event is provided
  2. tests

@RichardW98 RichardW98 force-pushed the kogito-7754 branch 8 times, most recently from 0153699 to f950e7a Compare January 15, 2024 17:52
ricardozanini
ricardozanini reviewed Jan 15, 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.

Great work! I think we are on the right path. I just added a few comments that we can make the implementation simpler and a note about triggers. Right on! Many thanks!

controllers/profiles/common/mutate_visitors.go Outdated Show resolved Hide resolved
controllers/profiles/common/mutate_visitors.go Outdated Show resolved Hide resolved
controllers/profiles/common/object_creators.go Outdated Show resolved Hide resolved
controllers/profiles/common/object_creators.go Outdated Show resolved Hide resolved
controllers/profiles/common/object_creators.go Show resolved Hide resolved
controllers/profiles/dev/states_dev.go Outdated Show resolved Hide resolved
controllers/profiles/prod/states_prod.go Outdated Show resolved Hide resolved
controllers/profiles/prod/deployment_handler.go Outdated Show resolved Hide resolved
controllers/profiles/common/ensurer.go Outdated Show resolved Hide resolved
controllers/profiles/common/mutate_visitors.go Outdated Show resolved Hide resolved
@RichardW98 RichardW98 mentioned this pull request Jan 16, 2024
Open
@RichardW98 RichardW98 force-pushed the kogito-7754 branch 2 times, most recently from e79cf76 to feef5e8 Compare January 17, 2024 03:52
@ricardozanini ricardozanini added this to the Knative Eventing Integration M1 milestone Jan 17, 2024
ricardozanini
ricardozanini reviewed Jan 17, 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.

Hi! I think we need a few movements here and there, but I can see that the feature is already there. Great work!

controllers/profiles/common/knative.go Outdated Show resolved Hide resolved
controllers/profiles/common/object_creators.go Outdated Show resolved Hide resolved
controllers/profiles/profile.go Outdated Show resolved Hide resolved
@RichardW98 RichardW98 force-pushed the kogito-7754 branch 2 times, most recently from 91c1535 to 6f6bb18 Compare January 17, 2024 20:06
@RichardW98
Copy link
Contributor Author

the extra knative status and state is removed. now user can edit the knative objs based on their needs. please review again

ricardozanini
ricardozanini reviewed Jan 18, 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.

One small last thing :P

controllers/profiles/common/object_creators.go Outdated Show resolved Hide resolved
@ricardozanini
Copy link
Member

Just verify the CI please.

@domhanak can you start the verification? 🙏

@RichardW98
Copy link
Contributor Author

/retest

@RichardW98 RichardW98 force-pushed the kogito-7754 branch 2 times, most recently from 623eb21 to df36511 Compare January 19, 2024 15:15
@RichardW98 RichardW98 force-pushed the kogito-7754 branch 2 times, most recently from 317ef96 to 816ab33 Compare January 23, 2024 14:12
@ricardozanini
Copy link
Member

ricardozanini commented Jan 23, 2024 •
edited
Loading

@RichardW98 please check the files generation:

make generate-all
make vet fmt

There's also unit tests failing because of the rebase.

@RichardW98
Copy link
Contributor Author

hi Ricardo, I just fixed the uni tests, please help check again

@RichardW98 RichardW98 force-pushed the kogito-7754 branch 2 times, most recently from 87d995a to 76d42ef Compare January 23, 2024 16:33
ricardozanini
ricardozanini approved these changes Jan 23, 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.

@wmedvede @RichardW98 do we have the follow up issues? If so, can you please link them here? 🙏

@RichardW98
Copy link
Contributor Author

RichardW98 commented Jan 24, 2024 •
edited
Loading

yes, I just created some followup issues, please find them under epic: https://issues.redhat.com/browse/KOGITO-9812 and the list is here:

KOGITO-10031 allow user customization on generated knative eventing resources
KOGITO-10032 create knative resources for DataIndex and JobService
KOGITO-10033 Support Multiple Sinks
KOGITO-10034 creation of sinks in Sonataflow
KOGITO-10035 convert the knative properties to immutable env var in deployment

please review these tickets and feel free to comment 🙂

@ricardozanini
Copy link
Member

@RichardW98 Thank you, but we are not using JIRA anymore. We used it for this one 'cause it was already opened. We should have issues within this repo and assign them to the milestone. Sorry about that, I'll open the issues here.

@RichardW98
Copy link
Contributor Author

@RichardW98 Thank you, but we are not using JIRA anymore. We used it for this one 'cause it was already opened. We should have issues within this repo and assign them to the milestone. Sorry about that, I'll open the issues here.

Thank you!

@RichardW98 RichardW98 force-pushed the kogito-7754 branch 2 times, most recently from fcfaddc to 20a7c9e Compare January 26, 2024 15:09
@ricardozanini ricardozanini mentioned this pull request Jan 26, 2024
Open
@ricardozanini
Copy link
Member

@RichardW98 can you take a look at the E2E failure?

@RichardW98 RichardW98 force-pushed the kogito-7754 branch 3 times, most recently from 5a36677 to e343613 Compare January 26, 2024 19:08
@ricardozanini
Copy link
Member

@RichardW98 we will put this on hold until we stabilize the features for the next two weeks.

@ricardozanini
Copy link
Member

@RichardW98 can you please rebase?

@ricardozanini ricardozanini merged commit 76ef902 into apache:main Feb 28, 2024
4 checks passed
rgdoliveira pushed a commit to rgdoliveira/kogito-serverless-operator that referenced this pull request Mar 11, 2024
@RichardW98 @rgdoliveira
[KOGITO-7754] create Knative eventing resources (apache#350)
9ca2f38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardozanini ricardozanini ricardozanini approved these changes

@wmedvede wmedvede wmedvede approved these changes

@domhanak domhanak Awaiting requested review from domhanak

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RichardW98 @ricardozanini @wmedvede