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

Change TaskType to be string in order to define custom task and taskMappers #3154

Merged
merged 5 commits into from
Oct 10, 2022

Conversation

manan164
Copy link
Contributor

@manan164 manan164 commented Aug 6, 2022

Pull Request type

  • Refactoring (no functional changes, no api changes)

Using String task to enable custom tasks and taskMapper to be implemented in future.

@manan164 manan164 changed the title String task type Change TaskType to be string in order to define custom task and taskMappers Aug 6, 2022
@james-deee
Copy link
Contributor

Just my 2 cents here, but loosening this type to a String doesn't seem quite like the correct thing to do here. Wonder if there's a better thing we can do to achieve your goal.

@manan164 can you give everyone a more descriptive, in depth explanation of what you are trying to get by doing this? I can't quite tell by your comment above.

@manan164
Copy link
Contributor Author

Hi @james-deee , The motivation behind this PR is to make the conductor task more extensible. Consider a case where we are using a conductor jar and want to add a few system tasks. As of now, there is no way to do that since TaskType is an enum and that can't be extended. This PR enables that functionality. So to add a system task two things are needed (assuming PR merged),

  1. Extend a WorkflowSystemTask
  2. Create TaskMapper for that Task
    Hope it is clear now. Happy to answer any further questions.

@apanicker-nflx apanicker-nflx added the type: maintenance Refactoring label Aug 20, 2022
@github-actions github-actions bot added the Stale label Oct 5, 2022
@Netflix Netflix deleted a comment from github-actions bot Oct 5, 2022
@apanicker-nflx apanicker-nflx merged commit 060a217 into Netflix:main Oct 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants