feat(GraphQL support): PoC for GraphQL support in datahub-frontend (Queries + Mutations) #2044
Conversation
| @@ -170,6 +170,7 @@ private ObjectNode userEntityProps() { | |||
| */ | |||
| @Nonnull | |||
|
|
|||
| // What is this good for?? | |||
There was a problem hiding this comment.
This comment will probably only add more confusion. Please open a ticket asking about this instead.
My guess is this is code that was exported from internal code.
There was a problem hiding this comment.
This shouldn't have been in here... will remove
| /** | ||
| * Constants relating to GraphQL type system & execution. | ||
| */ | ||
| public class Constants { |
There was a problem hiding this comment.
Nit: Generally try to avoid overly broad classes like this. Imagine this file 10 years from now being 500 lines of ungrouped constants.
Alternatives:
- (Preferred) A lot of this seems inference-able from the actual data models. Probably fine for a PoC, but for final code consider refactoring. Maybe contribute something to GMA to help out with this.
- Have a
TypeNames,FieldNames, etc enums or classes to group these constants.
There was a problem hiding this comment.
"Maybe contribute something to GMA to help out with this." -- Not sure I'm following this suggestion. Are you proposing we use the Rest.li API models exposed by the GMA API to reference particular fields?
Ideally we can find a way to auto-gen POJOs that included easy access to field names and then remove many of these....
Agree that in the long term this could grow unwieldy, but I generally opt for simplicity + centralization until natural boundaries have time to emerge. IMO, the scoping of this class to the GraphQL package is sufficient for now... I'm not super committed though, open to parsing apart if you feel strongly
| @@ -9,6 +9,7 @@ source frontend.env | |||
| set +a | |||
|
|
|||
| export JAVA_OPTS=" | |||
| -agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005 | |||
There was a problem hiding this comment.
Nit: again, remove for final submission.
jjoyce0510
changed the title
jjoyce0510
changed the title
| return DataLoader.newDataLoader(corpUserBatchLoader); | ||
| } | ||
|
|
||
| private DataLoader<String, com.linkedin.dataset.Dataset> createDatasetLoader() { |
There was a problem hiding this comment.
Extract these into dedicated classes.
|
Closing this, since we've moved on from the POC to actual implementation. |
This PR includes a proof-of-concept implementation of GraphQL in datahub-frontend. It is related to the open GraphQL RFC PRs:
Once the details in the RFC are agreed upon, we can work to productionalize the first chunk of GQL work using this PR.
Checklist