Skip to content
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

Add range to topic directive with range data fetcher #62

Merged
merged 26 commits into from
Sep 15, 2022

Conversation

raminqaf
Copy link
Contributor

Closes #57 and #59

Copy link
Contributor

@torbsto torbsto left a comment

Choose a reason for hiding this comment

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

I think it would be nice to also have some basic validation rules for the range arguments, e.g. only possible with list return type, key field must be set etc.

@raminqaf
Copy link
Contributor Author

@torbsto isn't that what you mean:

@Override
public boolean isValid(final TopicDirectiveContext context) {
return context.getTopicDirective().hasKeyArgument()
&& context.getTopicDirective().hasRangeFrom()
&& context.getTopicDirective().hasRangeTo()
&& context.isListType()
&& context.getEnvironment().getElement().getArguments().size() == 3
&& !context.getParentContainerName().equals(GraphQLUtils.SUBSCRIPTION_TYPE);
}

@torbsto
Copy link
Contributor

torbsto commented Aug 25, 2022

No, this checks if the DataFetcher can be applied to the given field. I'm talking about validation rules, that detect incorrect usages of the topic directive as implemented in https://github.com/bakdata/quick/tree/master/gateway/src/main/java/com/bakdata/quick/gateway/directives/topic/rule/validation

@raminqaf raminqaf marked this pull request as ready for review September 6, 2022 09:15
@raminqaf raminqaf requested a review from torbsto September 6, 2022 09:15
…y/add-range-data-fetcher

� Conflicts:
�	common/src/main/java/com/bakdata/quick/common/api/client/PartitionedMirrorClient.java
�	common/src/main/java/com/bakdata/quick/common/api/model/mirror/MirrorHost.java
�	gateway/src/main/java/com/bakdata/quick/gateway/fetcher/MirrorDataFetcherClient.java
@raminqaf raminqaf requested a review from torbsto September 7, 2022 11:26
@raminqaf raminqaf requested a review from torbsto September 8, 2022 12:21
@raminqaf raminqaf requested a review from torbsto September 9, 2022 12:17
@raminqaf raminqaf merged commit c8778ce into master Sep 15, 2022
@raminqaf raminqaf deleted the feature/gateway/add-range-data-fetcher branch September 15, 2022 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rangeFrom and rangeTo to @topic directive
2 participants