Skip to content

Allow graphql schema export to use a canonical representation #439

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

Merged
merged 2 commits into from
Mar 31, 2019

Conversation

garyd203
Copy link
Contributor

When we use the graphql_schema management command, the output can vary from run to run depending on arbitrary factors (because there is no guarantee made about the order used to output JSON dictionary keys). This makes it difficult to compare two schema's at different points in time.

We address this by including a new canonical flag to the command, which uses standard json.dump funcitonality to sort dictionary keys and force pretty-printed output.

@coveralls
Copy link

coveralls commented May 17, 2018

Coverage Status

Coverage increased (+0.2%) to 94.533% when pulling 6c1a301 on garyd203:canonical-schema into ea2cd98 on graphql-python:master.

@firaskafri
Copy link
Collaborator

@garyd203 Interested to update this pull request?

@garyd203
Copy link
Contributor Author

Yes @firaskafri, hopefully in the next day or two

@garyd203
Copy link
Contributor Author

I have rebased this now @firaskafri

@firaskafri
Copy link
Collaborator

Thank you @garyd203!

@danpalmer
Copy link
Collaborator

Given that the current behaviour is essentially defined by dict ordering which until recently has been explicitly something that shouldn’t be depended on, I don’t think this is really breaking anything.

I’m strongly in favour of remove this as it simplifies the code significantly.

It may be worth keeping a brief note in the documentation to say that keys are sorted as it’s a nice thing to be able to rely on now!

@jkimbo
Copy link
Member

jkimbo commented Mar 26, 2019

Completely agree with @danpalmer - this behaviour should be the default.

Copy link
Collaborator

@danpalmer danpalmer left a comment

Choose a reason for hiding this comment

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

Nice one. Thanks for the changes!

@jkimbo jkimbo merged commit fcc3de2 into graphql-python:master Mar 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants