feat: add graphql cli #131
Conversation
Addresses: strimzi#37 Signed-off-by: Ramakrishna Pattnaik <[email protected]>
PR ReportBundle SizesView bundle sizes for 'client'
Overall bundle size change: -71.61%View bundle sizes for 'server'
Overall bundle size change: -31.87%Test CoverageView test coverage
Triggered by commit: 4a5d063 |
|
To generate the entity model, two libraries were explored: graphql-cli and graphql-code-generator. The various ways to load schema have been discussed below:
These approaches would have been fine for development bit could cause inconsistencies in case strimzi-admin schema gets changed/modified.
Managing github modules can be cumbersome.
Github loader is able to download schema associated with the tag but it needs a Github personal access token which has to be passed as an environment variable.
This gets the work done for now, it requires to pass the commit ID of the tag. However we might need to fallback to a traditional method as pointing to a different github repo does not count as best practice. |
| @@ -0,0 +1,12 @@ | |||
| schema: https://raw.githubusercontent.com/rkpattnaik780/enmasse/14c4e82a0c13370d5e14a74669982e1a1105a0ba/console/console-init/ui/mock-console-server/schema.graphql | |||
There was a problem hiding this comment.
Do we have actual schema available somewhere? This looks like enmasse.
There was a problem hiding this comment.
Agreed - worth getting one into https://github.com/strimzi/strimzi-admin as a part of this issue?
There was a problem hiding this comment.
This schema can then be used/referenced in our MockApi too (in effect completing #50 )
There was a problem hiding this comment.
Do we have actual schema available somewhere? This looks like enmasse.
yes, This is enmasse schema. I suggested it for testing purpose. Later we can point out the actual schema.
There was a problem hiding this comment.
@matthew-chirgwin Do you have some barebones of the schema. Can I take look on it.
Little bit context. I'm involved in GraphQL spec and working on couple other standards like https://graphql-crud.org
We working on internal company standards for GraphQL schema and being involved in the process for creating schema for strimzi UI will be helpful..
There was a problem hiding this comment.
Sorry for the delay in replying @wtrocki - we have a very barebones/placeholder schema here:
strimzi-ui/server/mockapi/data.ts
Line 7 in e449946
There was a problem hiding this comment.
@wtrocki I am looking at defining the schema and would be interested in chatting to you about standards around things like identity, paging, etc. Shall I start a conversation in the strimzi-ui-dev slack channel?
There was a problem hiding this comment.
Yes. Feel free to ping me anywhere.
As for standards we have been using (and promoting) https://graphqlcrud.org
This comes with generators to reduce boilerplate etc. I have been demoing this already to some folks using
https://github.com/wtrocki/strimzi-experiments
See model.graphql as input and finalSchema which is the output. There is numerous benefits of using that format I can chat about
| ./generated/entityModelConstants.ts: | ||
| plugins: | ||
| - typescript | ||
| - typescript-react-apollo |
There was a problem hiding this comment.
some extra flags might be needed. HOC vs Hooks etc.
|
Amazing work. From my side to move this next step further it will be good to bring the actual graphql endpoint and see if we can generate hooks. |
There was a problem hiding this comment.
Hi @rkpattnaik780 / @wtrocki ! This looks interesting - is the idea that a (develop) time tool (graphql codegen?) generates the model from the schema (which is then imported/used by the client side)? If so, I will have some follow up questions
| @@ -0,0 +1,12 @@ | |||
| schema: https://raw.githubusercontent.com/rkpattnaik780/enmasse/14c4e82a0c13370d5e14a74669982e1a1105a0ba/console/console-init/ui/mock-console-server/schema.graphql | |||
There was a problem hiding this comment.
Agreed - worth getting one into https://github.com/strimzi/strimzi-admin as a part of this issue?
| @@ -0,0 +1,12 @@ | |||
| schema: https://raw.githubusercontent.com/rkpattnaik780/enmasse/14c4e82a0c13370d5e14a74669982e1a1105a0ba/console/console-init/ui/mock-console-server/schema.graphql | |||
There was a problem hiding this comment.
This schema can then be used/referenced in our MockApi too (in effect completing #50 )
|
Yes. Schema first development in the glance. Codegen generates types - Makes server side resolvers fully typed and Client side types and even data layer. Practically sealing entire API contract. The best pattern for working is to snapshot server-side schema in the repo to not get exposed by "Friday Evening PR merge" problems etc. - Getting schema updates from the server when requested/released rather than using github master |
GraphQL CLI can create mock out of the box. |
|
Sorry for the delay in following up @rkpattnaik780 / @wtrocki - thanks for answering my first set of questions. As is today, we have code which will introspect the backend to figure out what the schema contains (at runtime). We also will have code which will discover what the user has authorization to do. As described in the docs, the intent of the entity model was to combine these two inputs into a 'yes'/'no' CRUD abstraction (+use case specific items, such as live updates (subscriptions) etc). What I am missing here (and please correct me/let me know the intent) is how a generated version of the schema will help provide this model. We introspect the server already, so we know what is there. We (will have) information about what the user can do - how were you seeing these pieces come together? |
Sounds complex but I think I understand what intention is.
Little bit confused. We do not generate schema - more like typings (if we would want we can generate client side queries but this will be overkill for such simple schema)
I had some chat with folks about where schema should land etc. Introspecting server has multiple drawbacks of snapshoting schema (which is being used very often - especially with schemas that have more than >100 types. |
|
@wtrocki @matthew-chirgwin - my reading of this PR is that it's creating TS types from a graphql schema for development purposes. So that we can nicely handle gql results in TS. While this is useful from a development perspective, I believe the intent of the entity model is instead a way to model the available capabilities in the system, based off the result of a graphql introspection. E.g - introspect shows there is a |
|
I'm not sure how you can create types at runtime in a strongly typed language. |
It is much more. Defacto standard for GraphQL clients really (More generic one to the Relay) . It creates an entire data access layer using client requirements specification (queries, mutations). It means that there is no need to deal with graphql in the code at all and the code is actually detached. It keeps code detached from actual client (you can swap from Apollo to URQL in 20 seconds) etc. Most of the GraphQL foundation members use this approach (either thru Apollo CodeGen or GraphQL Code Generator)
I understand what we are saying, but it is not clear what requirement pushed us to have that architecture and also do not see why architecture is blocking what we want to do here. From my experience it looks like we are using introspection in production which has some drawbacks, but still if there is reason to do so.. why not.. I do not want to change the discussion from: "do we need codegen" to "is this architecture valid" because I'm not officially involved in the project, but in my understanding, there is no real blocker for having this PR merged. |
|
Major preface: I do not want to stifle the discussion, or options required to provide the entity model in any way. If this is part of an approach to achieve what is requested in #37 then I am all for understanding/discussing it. It sounds to me like there are some very crossed wires here. From your description @wtrocki it appears that this tooling will in effect replace/supersede our usage of Apollo in the client side by generating code to interact/access the backend based on a referenced schema. The reason we went with introspection at runtime is we want to use the available schema as a mechanism (in combination with other sources of data) to enable and disable features. That is the intent of #37 - a model to combine results from introspection, authorisation etc, to provide a simple Yes/No answer to if a type, field or operation are possible for the UI to use were needed. Eg, if no ‘Topic’ type exists on the backend, we should not show any topic pages/data/UI etc. I am curious as well, can this code generation approach deal with things like schema federation? Again, a reason we went with a runtime check here is to allow flexibility. Strimzi-admin today will host a GQL api which will service just topic information - but this could be (optionally) extended in due course, and I expect modules (say for metrics) would be federated into the overall schema if there was ‘metrics’ enabled/deployed as a part of a Strimzi deployment. |
|
My understanding of this issue is to generate an entity model. This is essentially a set of typescript types/interfaces that represent the schema. This cannot be generated at runtime, we have to code against it. This is somewhat orthogonal to introspection which is to determine if the capability exists at runtime. Or were you thinking to go with a detyped entity model? I would avoid schema federation unless it's critical for the functional requirements. It is complex, causes some things to not work well, and will slow the UI down. |
The introspection is already done. This should be combined with the entity model to determine what can be rendered. |
|
Agreed. For sake of the discussion/clarity, the original ask of the issue is to write a piece of code (at develop time), called at runtime which in effect reduces the result of introspection, authorization etc to an entity (and operations on that entity) centric view for the UI to make use of. Using the example of the topics page, if the introspection result shows no mutation to create, no 'create topic' button would be shown. Similarly, if the mutation was there, but the user doesnt have the authoization to create topics, again, the button would not be shown. Having said that, there are of course other ways of achieving the same effect. What I fail to see here is how what this PR contains is/would be used to achieve the outcome of this issue. |
There are much better (and simpler mechanisms to achieve that with GraphQL).
Introspection advisably would be development artifact. Happy to discuss. Generally, the more we diverge from how graphql is normally being used in the big players and move from standards the less possibilities we will have in the future to mitigate problems like performance etc.
Ok. Then it would be good to put some public proposal for this and look for possible options.
Schema federation for me will be different topic Based on my understanding Node.js gateway needs to be more router than federation gateway as all servers schema will be the same (with some subsets of features/enablement etc.) In Red Hat we have our own high-performance GraphQL Federation solution: We also had deployed Apollo Federation etc. |
|
+100 to what @pmuir was saying. To be clear I'm here as a community member not as Red Hatter. I would like to be able to point out my colleague who written introspection standard for GraphQL to some thread without much noise etc. so he can give us independent view. My take can be opiniated as I have been writing things certain way and build some Node libs etc. |
|
I've re-read the original description of this - https://github.com/strimzi/strimzi-ui/blob/master/docs/Architecture.md#entity-model - and I think some of confusion may be that what the architecture calls a "entity model" is typically called a data access layer (e.g. https://en.wikipedia.org/wiki/Data_access_object) or a repository, whilst typically the "entity model" refers to entities and their relationships (e.g. https://en.wikipedia.org/wiki/Entity%E2%80%93relationship_model). |
|
Leaving to one side the debate about whether the data access is cocrrect. How are we specifying the entities (e.g. |
|
Given the discussion in the PR and the original intent, I believe the next steps are as follows:
|
Addresses: #37
Signed-off-by: Ramakrishna Pattnaik [email protected]