Skip to content

DependentResource CDI awareness #1038

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
scrocquesel opened this issue Mar 15, 2022 · 9 comments
Closed

DependentResource CDI awareness #1038

scrocquesel opened this issue Mar 15, 2022 · 9 comments
Assignees
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@scrocquesel
Copy link
Contributor

I think JOSDK will need some kind of support for proper integration with CDI in Quarkus.

Actually, I find out :

@scrocquesel scrocquesel changed the title CDI awareness DependentResource CDI awareness Mar 15, 2022
@metacosm
Copy link
Collaborator

Yes, the resource type is a problem that I haven't gotten around to work on.

For the DependentResourceSpec issue, the first implementation had a configuration object that got removed for the standalone version. If I'm being completely honest, I would have rather not bother with the standalone use case for now.

@metacosm metacosm self-assigned this Mar 15, 2022
@scrocquesel
Copy link
Contributor Author

scrocquesel commented Mar 15, 2022

If I'm being completely honest, I would have rather not bother with the standalone use case for now.

Yep, but without a CDI aware DependentResourceManager, you can't use annotation.
Either, there is factory strategy configurable on @dependent (like mapstruct does), or the DependentResourceManager should be refactored to extract the setup responsability to an higher class and keep the reconcile/cleanup logic to an abstract class. Thus Quarkus can provide an instance with dependent provided at build time ?

@metacosm
Copy link
Collaborator

Yes, what I'm considering is delegating the actual DependentResource creation to a factory that would be configured via ConfigurationService with a default implementation being the current one while the Quarkus implementation would create beans.
For the resource type, I'm thinking of making it a method on DependentResource and make it explicit. The default implementation doesn't work in all instances and, hopefully, we should be able to have a no reflection implementation in Quarkus using synthetic beans for the DependentResources.

@metacosm
Copy link
Collaborator

What should the default CDI scope be for the dependents? I'm leaning on ApplicationScoped at the moment but could be convinced otherwise…

@metacosm
Copy link
Collaborator

Another question is how do you distinguish between dependents of the same type (e.g. your operator needs to have two dependents of type Deployment which might or might not be implemented by the same class)…

@scrocquesel
Copy link
Contributor Author

What should the default CDI scope be for the dependents? I'm leaning on ApplicationScoped at the moment but could be convinced otherwise…

That's fine for me, there isn't much other choice to be used. There isn't such a thing like PerReconciliation scope.

Another question is how do you distinguish between dependents of the same type (e.g. your operator needs to have two dependents of type Deployment which might or might not be implemented by the same class)…

Good question, I have exactly the same one trying to add hashmap to the ManagedDependentResourceContext to store per dependent ReconcileResult to be retrieve in the reconciler. I though of DependentResourceSpec as a key but it is not easy to retrieve it from the reconciler.
Note that we already have the issue in ManagedDependentResourceContext for getDependentResource.

@scrocquesel
Copy link
Contributor Author

Following up #1043, there is another class type issue in ManagedDependentResourceContext::getDependentResource.

See quarkiverse/quarkus-operator-sdk#284

@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 May 20, 2022
@github-actions
Copy link

github-actions bot commented Jun 4, 2022

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

@github-actions github-actions bot closed this as completed Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

2 participants