Make Conductor model extendable

[andrea11]

Hello, I was thinking to "change" (more precisely extend) my data model of conductor. Today I could fork the project and edit directly. But I was thinking to a more generic approach, using in the interface only generics that extend your base model. For example:

public interface ExecutionDAO<T extends Task, W extends Workflow> {
    List<T> getPendingTasksByWorkflow(String taskName, String workflowId);
    W getWorkflow(String workflowId, boolean includeTasks);

What do you think? Is there a better approach to avoid this huge refactoring?

[Jiehong]

Hello @andrea11,

I think something more in this vein might make more sense if I understand your point well. Here with the ExecutionDAO as well:

public interface ExecutionDAO {
  <T extends Task> List<T> getPendingTasksByWorkflow(String var1, String var2);

  <T extends Task> List<T> getTasks(String var1, String var2, int var3);

  <T extends Task> List<T> createTasks(List<T> var1);

  <T extends Task> void updateTask(T var1);
}

As a T should always be a valid Task in this case, very few issues should occur I reckon, but it's a breaking change, indeed.

What do you think?

[Jiehong]

but I'd be curious to see if a Task is a valid T, but that might be the case

[Jiehong]

However, for now, if you implement DAOs with a T that actually extends your model, you should be fine with upcasting/downcasting, even though it's ugly.

[Jiehong]

but I'd be curious to see if a Task is a valid T, but that might be the case

Well, after testing, it seems it is indeed perfectly valid:

public class Test {
  private <T extends String> void handle(final T input) {
    return;
  }

  private void start() {
    this.handle("Hello");
  }
}

[andrea11]

@apanicker-nflx @kishorebanala @naveenchlsn Any suggestion about it? It could be something that could be accepted by/interesting for the community? Me and my team were thinking to prepare a first draft PR to give a better overview.

Waiting for your advice 😃 Thanks

[naveenchlsn]

@andrea11 Can you please let me know what are the benefits of run-time binding of data models? What functionality in the service classes/api's are we aiming to achieve with this change?

[andrea11]

Hello @naveenchlsn, The purpose here is to make Conductor even more extensible. For example, we would like to add new properties to our pojos (task and workflow), but we do not want to merge these changes here because they are too specific to our own use cases. We could fork Conductor, but it would be nicer to allow users to extends their own model, making the interfaces accepting it. Conductor will still continue to offers (and uses) basic model, but it would be also possible to extend it and replacing with a more complex one.

Per se, this change will not add any new features on services/apis and it will be compatible with the current model

[apanicker-nflx]

@andrea11 and @AlexVinogradov Thank you for your suggestions and contributions. This is breaking change on the interface contracts and anyone extending the current interfaces would need to update their codebases. While this is useful and improves the extensibility of Conductor, we believe that this change should be made as part of a major version release. We will evaluate this and look into integrating this feature into the next major release (v3.0)

[github-actions[bot]]

This issue is stale, because it has been open for 45 days with no activity. Remove the stale label or comment, or this will be closed in 7 days.

[github-actions[bot]]

This issue was closed, because it has been stalled for 7 days with no activity.

[Jiehong]

Version 3 happened, but it was not changed… (thanks github-bot I guess?)

@apanicker-nflx could you please re-open this?