Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Feature: Need for Task status listener #3623

Merged
merged 13 commits into from
Jun 17, 2023

Conversation

charybr
Copy link
Contributor

@charybr charybr commented May 23, 2023

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes (Please run ./gradlew generateLock saveLock to refresh dependencies)
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Describe the new behavior from this PR, and why it's needed
Issue #3587

Need for Task status listener
When the status of a task changes, we need a mechanism to notify. Just like we have WorkflowStatusListener interface, we can have TaskStatusListener interface.

Alternatives considered

Describe alternative implementation you have considered

charybr pushed a commit to charybr/conductor-community that referenced this pull request May 24, 2023
@manan164
Copy link
Contributor

Hi @charybr , thanks for the contribution.
Following are my thoughts,

  1. We can put only the interface and stub implementation in the main repo and the different types of implementation in the community repo.
  2. Here I see when the task gets scheduled we are notifying the external system about that. However, the status update listener can be generalized for all task statuses like SCHEDULED, IN_PROGRESS, FAILED, COMPLETED etc

thoughts @v1r3n @c4lm @apanicker-nflx ?

@charybr
Copy link
Contributor Author

charybr commented May 25, 2023

Thanks very much Manan! for the response. Please find my comments inline.

Hi @charybr , thanks for the contribution. Following are my thoughts,

1. We can put only the interface and stub implementation in the main repo and the different types of implementation in the community repo.

This is correct, just like in the case of WorkStatusListener I am adding interface and stub in main repo. Implementation is in the form of Webhook module in conductor-community repo (there is a separate PR - Netflix/conductor-community#228)

2. Here I see when the task gets scheduled we are notifying the external system about that. However, the status update listener can be generalized for all task statuses like `SCHEDULED, IN_PROGRESS, FAILED, COMPLETED` etc

Soon i will add other task status listener methods.

Raghavendra Chary B added 2 commits May 26, 2023 20:49
@charybr
Copy link
Contributor Author

charybr commented May 26, 2023

Hi @manan164, added status listener methods for all other statuses. Please review.
4ff8a49
7035276

@v1r3n v1r3n merged commit e68dee6 into Netflix:main Jun 17, 2023
charybr added a commit to charybr/conductor-community that referenced this pull request Jul 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants