-
Notifications
You must be signed in to change notification settings - Fork 95
graphene/graphql-core v3 upgrade #6478
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
Conversation
6258587 to
234a4df
Compare
b4ee741 to
dc47ded
Compare
|
Another change I've found is with whereas this will not: you'll get an error, the None/null is now carried. And we have a lot of scripts that set args to None in the options.. Slowly working through them.. They'll need to be a valid default in the options. |
990d9dc to
5acb256
Compare
|
LGTM 🚀 8.4.0 is hopefully imminent (remove + skip mode), so we're probably aiming for 8.5.0 (cylc-uiserver 1.7.0). Will need to have a play with the cylc-ui forms to see what the impact of the enum changes are. Hopefully, we can keep cylc-uiserver compatible with older cylc-flow schedulers (perhaps using some back-compat shims in cylc-uiserver if needed).
Good spot. Will need to make sure the UI does the right thing. |
e3431c6 to
3c36e56
Compare
|
Just made some more changes; covered more code, fixed a few issues..
Well, the sync is just protobuf.. And I haven't noticed anything other than those defaults from the GraphQL client end. |
|
Once we merge this, we're committed to releasing the uis/ui components in the same minor release. Hopefully, we can target 8.5.0, created a milestone to assign this to. Note to reviewers, don't merge into master until 8.4.0 has been released. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
b0775d4 to
97220c8
Compare
97220c8 to
9d36086
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
Have tested out the stop flow arg change, all good. |
f667dd9 to
7ce596b
Compare
Co-authored-by: Oliver Sanders <[email protected]>
Co-authored-by: Oliver Sanders <[email protected]>
* Revert GQL execution error handling * undo fail example --------- Co-authored-by: David Sutherland <[email protected]>
Co-authored-by: Oliver Sanders <[email protected]>
Co-authored-by: Oliver Sanders <[email protected]>
7ce596b to
3e2769b
Compare
|
Glad I spent a little more time on this (thanks @MetRonnie ); I've significantly simplified the |
|
Edit: pushed a change 🎉 |
- Remove duplicate param from one test - Reinstate possibly-mistaken deleted param from another
|
🎉 |
partially addresses cylc/cylc-uiserver#333
sibling to
cylc/cylc-uiserver#672
cylc/cylc-ui#2091
This is the first part described here:
cylc/cylc-uiserver#333 (comment)
The second, and arguably trickier, part will be the sibling at the UIS.
There were some breaking changes to contend with (mainly around more support for Enums):
https://github.com/graphql-python/graphene/wiki/v3-release-notes
But the most involved part was re-implementing the null stripping, but some time reading the graphql-core I found some tools to help, simplifying the code.
Note:
There are some incompatibilities (i.e. with Enums) on the first version (3.0.0):https://github.com/graphql-python/graphene/wiki/v3-release-notes
(and perhaps beyond)
The coverage will be lower due to some items only being used at the UIS (at present).
Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).