improve: dependent configuration improvements - context independent #2389
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci
bot
added
the
do-not-merge/work-in-progress
csviri
changed the title
This was referenced May 20, 2024
This was
linked to
issues
May 21, 2024
openshift-merge-robot
added
the
needs-rebase
csviri
force-pushed
the
independent-config
branch
from
41e50b1
to
5b6a3dd
Compare
openshift-merge-robot
removed
the
needs-rebase
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
metacosm
requested changes
May 23, 2024
...o/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java
Outdated
Show resolved
Hide resolved
csviri
force-pushed
the
independent-config
branch
from
a04f49c
to
8bd6a2e
Compare
csviri
force-pushed
the
independent-config
branch
from
8bd6a2e
to
272449b
Compare
openshift-merge-robot
added
the
needs-rebase
csviri
force-pushed
the
independent-config
branch
from
02a1158
to
3e275b7
Compare
openshift-merge-robot
removed
the
needs-rebase
csviri
force-pushed
the
independent-config
branch
from
8d88356
to
20a12e2
Compare
openshift-merge-robot
added
the
needs-rebase
Signed-off-by: Attila Mészáros <csviri@gmail.com>
csviri
force-pushed
the
independent-config
branch
from
20a12e2
to
f7497ff
Compare
openshift-merge-robot
removed
the
needs-rebase
Signed-off-by: Attila Mészáros <csviri@gmail.com>
csviri
force-pushed
the
independent-config
branch
from
ead1f2e
to
bd18204
Compare
Signed-off-by: Chris Laprun <claprun@redhat.com>
csviri
commented
Jun 19, 2024
| if (kubernetesDependentResourceConfig != null | ||
| && kubernetesDependentResourceConfig.informerConfig() != null) { | ||
|
|
||
| var configBuilder = informerConfigurationBuilder(context); | ||
| kubernetesDependentResourceConfig.informerConfig().updateInformerConfigBuilder(configBuilder); |
There was a problem hiding this comment.
Note that this is not good this way since overrides the name even if it is null in the informer config
metacosm
reviewed
Jun 19, 2024
| @@ -111,11 +111,12 @@ static Set<String> ensureValidNamespaces(Collection<String> namespaces) { | |||
| * | |||
| * @return a Set of namespace names the associated controller will watch | |||
| */ | |||
| default Set<String> getEffectiveNamespaces(ConfigurationService configurationService) { | |||
| default Set<String> getEffectiveNamespaces(ControllerConfiguration<?> controllerConfiguration) { | |||
There was a problem hiding this comment.
This really isn't ideal: an interface should not reference one of its children in its API.
There was a problem hiding this comment.
Yes, but this is just an utility method effectively. I think the problem is more about that ControllerConfiguraiton extends ResourceConfiguration, since Controller is not really a resource resource. It should rather reference a resource configuration (the primary resource), but not sure if we want to change that at this point.
Maybe in a different PR we can take a look on this.
| return name; | ||
| } | ||
|
|
||
| public boolean inheritsNamespacesFromController() { |
There was a problem hiding this comment.
Why this new method? What's the difference with followControllerNamespaceChanges?
There was a problem hiding this comment.
The follow follows changes if controller namespaces changes dynamically in runtime, the inherit is about the initial value, if the name is confusing we can change it.
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
metacosm
approved these changes
Jun 19, 2024
metacosm
added a commit
that referenced
this pull request
Jul 8, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
metacosm
added a commit
that referenced
this pull request
Jul 9, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
metacosm
added a commit
that referenced
this pull request
Jul 12, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
csviri
added a commit
that referenced
this pull request
Aug 8, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
csviri
added a commit
that referenced
this pull request
Aug 15, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
metacosm
added a commit
that referenced
this pull request
Aug 29, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
csviri
added a commit
that referenced
this pull request
Sep 20, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
metacosm
added a commit
that referenced
this pull request
Oct 10, 2024
…2389) Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.