feat: ResourceIDMapper for external event sources, external dependents and bulk dependents #3020
Conversation
…pper Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Will check if this could be elegantly extended to bulk resources Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
…s and bulk dependents Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
docs/content/fileList.txt
Outdated
There was a problem hiding this comment.
This is a deletion of empty file, it is not needed.
...or-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java
Outdated
Show resolved
Hide resolved
...or-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java
Outdated
Show resolved
Hide resolved
...or-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java
Show resolved
Hide resolved
...-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDProvider.java
Outdated
Show resolved
Hide resolved
...-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDProvider.java
Outdated
Show resolved
Hide resolved
...-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDProvider.java
Outdated
Show resolved
Hide resolved
...-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDProvider.java
Show resolved
Hide resolved
…tor/processing/ResourceIDMapper.java Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
…tor/processing/ResourceIDMapper.java Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
…tor/processing/ResourceIDProvider.java Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
…tor/processing/ResourceIDProvider.java Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
docs/content/en/docs/documentation/dependent-resource-and-workflows/dependent-resources.md
Outdated
Show resolved
Hide resolved
Signed-off-by: Chris Laprun <metacosm@gmail.com>
There was a problem hiding this comment.
Several comments:
- As previously mentioned, I don't think that putting the responsibility of providing the external resource id on the external resource class is a good idea. We need to decide which design to go with now because this is already a breaking change and it shouldn't change again soon after, if we decide that putting the id "generation" on the dependent class (for example) is a better design later.
- The documentation for this feature is rather lacking and doesn't really explain the concepts or how to use things so it needs to be improved, imo.
That is completely optional, but allows us to add meaningful default implementation for the mapper. What alternative would you propose?
Not shure what do you mean by this. Can you pls elaborate?
Yeah, in general we have quite badly documented, features around it, would rather followup with a comprehensive rework on the docs. |
it is by definition single, so namespace scoping is out of the picture, also does not confuse this way with some hidden intention in the background Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
| */ | ||
| static <R> ResourceIDMapper<R, String> singleResourceResourceIDMapper() { | ||
| return r -> "josdk:mapper:id"; | ||
| return r -> "id"; |
There was a problem hiding this comment.
The value should be irrelevant and we should rather use a value that has not risk of conflicting with an identifier a user would create.
There was a problem hiding this comment.
Will have to upgrade upgrade the javadocs, there is no situation where there can be a collision. Assuming that this is by definition not usable for BulkDependentResources - will add javadoc.
If you see some situation where it actually can lead to a collision pls describe it.
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
No description provided.