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

Use Graphql extra feature #135

Merged
merged 3 commits into from Oct 19, 2021
Merged

Conversation

chadell
Copy link
Contributor

@chadell chadell commented Oct 14, 2021

No description provided.

glennmatthews
glennmatthews previously approved these changes Oct 14, 2021
Copy link
Contributor

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

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

I'm missing context as to what issue 399 had to do with not enabling GraphQL, but this looks reasonable. Would it make sense to add a couple of graphql unit test cases though?

@dgarros
Copy link
Contributor

dgarros commented Oct 14, 2021

I'm missing context as to what issue 399 had to do with not enabling GraphQL, but this looks reasonable. Would it make sense to add a couple of graphql unit test cases though?

@glennmatthews before #399, a GraphQL type generated by @extra_features inside a plugin wouldn't include a filterset so the workaround was to define the graphql types manually

@chadell if you are enabling @extra_features on these models, you need to also delete the corresponding GraphTypes defined in https://github.com/nautobot/nautobot-plugin-circuit-maintenance/blob/develop/nautobot_circuit_maintenance/graphql/types.py
Looks like could delete the file altogether

@chadell
Copy link
Contributor Author

chadell commented Oct 15, 2021

thanks @dgarros , this was actually my thought about it but was not sure.
I will try to remove the current GraphTypes if they are not needed

@chadell
Copy link
Contributor Author

chadell commented Oct 19, 2021

I verified that using extra_features was colliding with the craft types:

nautobot_1       | 05:11:28.367 WARNING nautobot.graphql.schema schema.py                 generate_query_mixin() :
nautobot_1       |   Unable to load schema type for the model "nautobot_circuit_maintenance.circuitmaintenance" as there is already another type registered under this name. If you are seeing this message during plugin development, check to make sure that you aren't using @extras_features("graphql") on the same model you're also defining a custom GraphQL type for.
nautobot_1       | 05:11:28.368 WARNING nautobot.graphql.schema schema.py                 generate_query_mixin() :
nautobot_1       |   Unable to load schema type for the model "nautobot_circuit_maintenance.circuitimpact" as there is already another type registered under this name. If you are seeing this message during plugin development, check to make sure that you aren't using @extras_features("graphql") on the same model you're also defining a custom GraphQL type for.

so removing non necessary code

@chadell
Copy link
Contributor Author

chadell commented Oct 19, 2021

@glennmatthews I've added testing for GraphQL.
As it was already approved (and only cleanup and tests have been added), I would move it forward when tests pass.

@chadell chadell merged commit 0cc9ed8 into develop Oct 19, 2021
@chadell chadell deleted the issue121-use-graphql-extra-feature branch October 19, 2021 06:04
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.

None yet

3 participants