Skip to content

Rename dependents to workflow in @ControllerConfiguration in v5 #1773

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.

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

Closed
csviri opened this issue Feb 16, 2023 · 15 comments · Fixed by #2274
Closed

Rename dependents to workflow in @ControllerConfiguration in v5 #1773

csviri opened this issue Feb 16, 2023 · 15 comments · Fixed by #2274

Comments

@csviri
Copy link
Collaborator

csviri commented Feb 16, 2023

This popped up lately, on a discussion of workflows it is very confusing that it is not in the domain language here:

https://github.com/java-operator-sdk/java-operator-sdk/blob/2fda82bca69fc2ea798171aaacd7d4e87ea9188b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java#L105-L105

would be good to rename that attribute to workflow.

Maybe even warp it into a @Workflow annotation for forward compatibiilty.

@csviri csviri added this to the 5.0 milestone Feb 16, 2023
@metacosm
Copy link
Collaborator

I don't really agree because dependents has a meaning in the context of operators while workflow is something that doesn't really exist in the domain… if anything should be renamed, it should be workflows, imo. Also, we've talked about dependents a lot in talks and presentations so it'd be more complex to rename as well.

@csviri
Copy link
Collaborator Author

csviri commented Feb 16, 2023

I'm talking about this one field in the annotation, nothing else.

So workflow exists, also there is a builder in standalone mode, to create workflow and orchestrate dependent resources. It a workflow above dependent resources.

@csviri
Copy link
Collaborator Author

csviri commented Feb 16, 2023

What do you mean workflow does not exists in the domain btw, there is a whole nice section about it, what is a workflow:
https://javaoperatorsdk.io/docs/workflows
:)

@metacosm
Copy link
Collaborator

What do you mean workflow does not exists in the domain btw, there is a whole nice section about it, what is a workflow:
https://javaoperatorsdk.io/docs/workflows
:)

What I mean is that workflow is not a concept traditionally associated with the operator domain.

@metacosm
Copy link
Collaborator

metacosm commented Feb 20, 2023

I'm talking about this one field in the annotation, nothing else.

I understand but to me, it still doesn't make sense to rename that field as what is being defined by it really are the dependents, not a workflow. There's an implicit workflow if people define conditions on the dependents but there might not be any such conditions. Essentially, when using this field, there will always be dependents but there might not be a workflow associated with them so it'd be weird to me to rename that field workflow… What might make sense, though, would be to rename the properties that define the conditions to maybe be more explicit.

@csviri
Copy link
Collaborator Author

csviri commented Feb 21, 2023

but there might not be a workflow

There is always a workflow create in the background. And even if there is no depends on relations or condition, just two or more dependents created in the background, and the fact that those are reconciled in parallel is actually a definition/responsibility of a workflow.

But the main problem I faced, and motivated the issue, is that I tried to explain to someone that how these workflows work on top of dependent resources on our example, and basically we have a nice definition of dependent resources and workflows and nice documentation. The workflows are nowhere to find in the domain language. So had to explain that the complex structure where I reference a bunch of dependent resources under dependeents={...} is actually that workflow that we talk about in the docs. So this is a quite obvious smell to me.

@metacosm
Copy link
Collaborator

I think that maybe we should just not talk about workflows as they are an implementation detail and just mention that you can create relations between dependents. The fact that there is something like a workflow that gets created by the implementation shouldn't make it necessary to bubble the concept up. For example, explaining dependents without any relations wouldn't make any sense if the attribute was named workflow… It probably shouldn't be a separate part in the documentation either, actually, but rather part of the documentation on dependents.

@csviri
Copy link
Collaborator Author

csviri commented Feb 21, 2023

These two concepts have clear boundaries. Remember, we have standalone dependents which are independent from workflows. Basically if anybody wan't to implement it's own logic in what order and/or under what condition reconcile resources can use standalone dependent resources, that is quite powerful, and makes dependent resources much more powerful. There is no reason to mix there depends on relation or conditions, since users do it in their own code (in case of standalone).

On the other hand we offer a quite generic way to handle ordering and conditional way to reconcile dependents. But that is limited, will never cover all cases. (That is why even extensions like after might needed in the future to cover more: #1774 ).

So workflows is a complementary feature to dependents, since it is very much make sense to use dependents without workflows.

On the other hand with annotation we create workflows. As mentioned, even parallelism and other features might be configurable within that annotation like:

@ControllerConfiguration( workflow = @Workflow( parallelism = 10,  // possible other configs fow workflow
       dependents={
           @Dependent(type=SampleDependent1.class), 
          @Dependent(type=SampleDependent1.class)}))

But we are defining there a workflow with annotation, always. And the problem is it is not called that way.

@csviri
Copy link
Collaborator Author

csviri commented Feb 21, 2023

actually might be even good to have a dedicated @Workflow annotation as above, for future compatibility.

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 23, 2023
@csviri csviri removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 23, 2023
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Jun 23, 2023
@csviri csviri removed the stale label Jun 23, 2023
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Aug 23, 2023
@csviri csviri removed the stale label Aug 23, 2023
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Oct 23, 2023
@csviri csviri removed the stale label Oct 23, 2023
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Dec 23, 2023
@csviri csviri removed the stale label Dec 23, 2023
@csviri csviri linked a pull request Jan 18, 2024 that will close this issue
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Feb 22, 2024
@csviri csviri removed the stale label Feb 22, 2024
@csviri csviri linked a pull request Mar 8, 2024 that will close this issue
@csviri csviri closed this as completed Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment