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

StartProgram Event Handling #15227

Merged
merged 1 commit into from
Jul 27, 2023
Merged

StartProgram Event Handling #15227

merged 1 commit into from
Jul 27, 2023

Conversation

codeNinjaDev
Copy link
Contributor

@codeNinjaDev codeNinjaDev commented Jul 10, 2023 •
edited
Loading

Builds on Event Reader Subscriber infrastructure to handle StartProgramEvents:
Based on #15223

@codeNinjaDev codeNinjaDev added the build Triggers github actions build label Jul 11, 2023
@codeNinjaDev codeNinjaDev force-pushed the feature/handler-extension branch 3 times, most recently from 002fd72 to 73db87b Compare July 11, 2023 18:05
@codeNinjaDev codeNinjaDev force-pushed the feature/cdap-20678-event-pr branch 2 times, most recently from fcda35c to adbc549 Compare July 11, 2023 20:32
@codeNinjaDev codeNinjaDev force-pushed the feature/handler-extension branch 4 times, most recently from 7730313 to 441d837 Compare July 11, 2023 22:15
@codeNinjaDev codeNinjaDev requested a review from chtyim July 11, 2023 22:17
@codeNinjaDev codeNinjaDev force-pushed the feature/handler-extension branch 2 times, most recently from 8f46328 to 0e7fd72 Compare July 12, 2023 20:49
masoud-io
masoud-io reviewed Jul 17, 2023
View reviewed changes
cdap-app-fabric/pom.xml Outdated Show resolved Hide resolved
cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java Outdated Show resolved Hide resolved
...ric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventReaderExtensionProvider.java Outdated Show resolved Hide resolved
...ric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventReaderExtensionProvider.java Outdated Show resolved Hide resolved
...ric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventReaderExtensionProvider.java Outdated Show resolved Hide resolved
cdap-app-fabric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventSubscriber.java Outdated Show resolved Hide resolved
cdap-app-fabric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventSubscriber.java Outdated Show resolved Hide resolved
cdap-app-fabric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventSubscriber.java Outdated Show resolved Hide resolved
cdap-app-fabric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventSubscriber.java Outdated Show resolved Hide resolved
cdap-app-fabric/src/main/java/io/cdap/cdap/internal/events/StartProgramEventSubscriber.java Outdated Show resolved Hide resolved
masoud-io
masoud-io approved these changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@masoud-io masoud-io left a comment

Choose a reason for hiding this comment

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

LGTM

masoud-io
masoud-io approved these changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@masoud-io masoud-io left a comment

Choose a reason for hiding this comment

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

LGTM. Couples of notes:

  1. Make sure to squash commits before merge.
  2. Make sure to submit after event handling infra PR.
  3. Make sure to get a second LGTM

@codeNinjaDev codeNinjaDev force-pushed the feature/handler-extension branch 2 times, most recently from 155900d to 83b4d93 Compare July 21, 2023 18:06
@codeNinjaDev
Copy link
Contributor Author

Tested in sandbox (without enabling event subscribing), deploying and running a pipeline is successful

@codeNinjaDev codeNinjaDev force-pushed the feature/handler-extension branch 2 times, most recently from 2b965e9 to f780ecb Compare July 25, 2023 18:04
Base automatically changed from feature/cdap-20678-event-pr to develop July 25, 2023 18:13
@codeNinjaDev codeNinjaDev merged commit 3a07df7 into develop Jul 27, 2023
5 checks passed
@codeNinjaDev codeNinjaDev deleted the feature/handler-extension branch July 27, 2023 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greeshmaswaminathan greeshmaswaminathan greeshmaswaminathan approved these changes

@chtyim chtyim Awaiting requested review from chtyim

@masoud-io masoud-io Awaiting requested review from masoud-io

Assignees
No one assigned
Labels
build Triggers github actions build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@codeNinjaDev @chtyim @masoud-io @greeshmaswaminathan