control-service: Add Data Job Executions to the graphql #265
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
kostoww
force-pushed
the
person/kostoww/implement-executions-in-graphql
branch
from
6a2ec3d to
e52d127
Compare
antoniivanov
requested changes
Sep 24, 2021
.../pipelines_control_service/src/main/java/com/vmware/taurus/service/graphql/GraphQLUtils.java
Outdated
Show resolved
Hide resolved
...ontrol_service/src/integration-test/java/com/vmware/taurus/datajobs/it/DataJobGraphQLIT.java
Show resolved
Hide resolved
...es_control_service/src/main/java/com/vmware/taurus/service/graphql/ExecutionDataFetcher.java
Show resolved
Hide resolved
.../pipelines_control_service/src/main/java/com/vmware/taurus/service/graphql/GraphQLUtils.java
Show resolved
Hide resolved
|
Looks pretty good to me . My comments are mostly minor things like fixing the documentation and making sure we've tested at good level. |
mivanov1988
reviewed
Sep 24, 2021
...l_service/src/main/java/com/vmware/taurus/service/graphql/model/ExecutionQueryVariables.java
Outdated
Show resolved
Hide resolved
...es_control_service/src/main/java/com/vmware/taurus/service/graphql/ExecutionDataFetcher.java
Outdated
Show resolved
Hide resolved
...es_control_service/src/main/java/com/vmware/taurus/service/graphql/ExecutionDataFetcher.java
Outdated
Show resolved
Hide resolved
...es_control_service/src/main/java/com/vmware/taurus/service/graphql/ExecutionDataFetcher.java
Outdated
Show resolved
Hide resolved
...es_control_service/src/main/java/com/vmware/taurus/service/graphql/ExecutionDataFetcher.java
Show resolved
Hide resolved
...nes_control_service/src/main/java/com/vmware/taurus/service/graphql/GraphQLDataFetchers.java
Outdated
Show resolved
Hide resolved
|
Thanks for doing this change! |
|
In order to validate if the API works you can put a new test in |
Testing: Local run of tests and manual testing, also added some new test cases in unit tests and some code change in Integration tests Signed-off-by: Plamen Kostov <[email protected]>
kostoww
force-pushed
the
person/kostoww/implement-executions-in-graphql
branch
from
e52d127 to
fe23af0
Compare
antoniivanov
approved these changes
Sep 27, 2021
mivanov1988
approved these changes
Sep 27, 2021
tpalashki
pushed a commit
that referenced
this pull request
Sep 29, 2021
Testing: Local run of tests and manual testing, also added some new test cases in unit tests and some code change in Integration tests Signed-off-by: Plamen Kostov <[email protected]>
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.
Why:
Job executions are important part of the data jobs details. They could be easily attached to the existing graphql endpoint in order to extend the capabilities for the consumers.
What is done:
This implementation is providing the ability for the users to attach executions details to each data job in graphql result list, by providing functionality to sort and page the results by native graphql query: eg. specifying execution field in GraphQL request and provide arguments, eg: executions(pageNumber: 1, pageSize: 5, filter: [...]) nested in the main jobs query. There are few limitation that are introduced:
Executions page limitation are the same as the parent query: currently (1 <= pageSize <= 100) and (1 <= pageNumber)
Execution page currently cannot be seen (like in the jobs list) cause its nested object field
Execution uses filtering object (same as parent query), but could not use filtering, only sorting (validated with check).
Filtering will require using Spring Data Specification API (in order not to load all data jobs for each job) and merge it with current GraphQL strategy field pattern
Testing:
Local run of tests and manual testing, also added some new test cases in unit tests and some code change in Integration tests.
(ref: #120)
Signed-off-by: Plamen Kostov [email protected]