Skip to content

All Resource Event Sources can handle multiple seconday resources for a primary resources #1169

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “No 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? No Sign in to your account

Merged
merged 20 commits into from
Apr 22, 2022

Conversation

csviri
Copy link
Collaborator

@csviri csviri commented Apr 21, 2022

No description provided.

@csviri csviri self-assigned this Apr 21, 2022
@csviri csviri changed the title WIP: All Resource Event Sources can handle multiple seconday resources for a primary resources All Resource Event Sources can handle multiple seconday resources for a primary resources Apr 21, 2022
@csviri csviri marked this pull request as ready for review April 21, 2022 12:27
@csviri csviri requested a review from metacosm April 21, 2022 12:27
@csviri
Copy link
Collaborator Author

csviri commented Apr 21, 2022

@metacosm will review some parts little later, and fix javadoc. But conceptually can be reviewed.


import java.util.function.Function;

public interface IDMapper<R> extends Function<R, String> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather have an explicit interface with an actual method which name we control rather than a Function extension. Easier to extend at a later time and more self-documenting than a random apply method in implementors, imo.
I'm also unsure about the purpose of this, actually. :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

@csviri csviri Apr 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed.

Can try to explain more in depth here in the javadoc why this needed. But basically we need to have the information if it's a new resource is a new version or just the same as an other. With key check if it referring to same object with equals we check if it changed or not.

csviri and others added 4 commits April 22, 2022 16:55
…tor/processing/event/source/ExternalResourceCachingEventSource.java

Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>
@metacosm metacosm force-pushed the all-multi-resource-es branch from a4054b8 to 8b48c1d Compare April 22, 2022 15:05
public Optional<Schema> fetchResource(MySQLSchema resource) {
return schemaService.getSchema(resource.getMetadata().getName());
public Set<Schema> fetchResources(MySQLSchema primaryResource) {
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this normal? If yes, can you add a comment explaining why, please?

Copy link
Collaborator Author

@csviri csviri Apr 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not used, delete it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! :)

@metacosm metacosm self-requested a review April 22, 2022 15:30
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 11 Code Smells

63.3% 63.3% Coverage
0.0% 0.0% Duplication

@csviri csviri merged commit 3c3c5bd into main Apr 22, 2022
@csviri csviri deleted the all-multi-resource-es branch April 22, 2022 15:37
No Sign up for free to join this conversation on GitHub. Already have an account? No Sign in to comment
Labels
None yet
Projects
None yet
2 participants