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

Raise an error when invalid sort keys are provided by a GraphQL client #4115

Merged
merged 2 commits into from
Mar 10, 2021

Conversation

kinow
Copy link
Member

@kinow kinow commented Mar 8, 2021

These changes close #4114

Transferred from Cylc UI Server repository as I realized the change had to be done in the Cylc Flow schema.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.
  • No dependency changes.

@kinow kinow added this to the cylc-8.0b0 milestone Mar 8, 2021
@kinow kinow self-assigned this Mar 8, 2021
@kinow
Copy link
Member Author

kinow commented Mar 8, 2021

Useful for clients like TUI, Cylc UI, and GraphiQL.

Screenshot from 2021-03-09 10-31-25

Screenshot from 2021-03-09 10-28-50

What happened in my case was that for Cylc UI I wrote a subscription deltas query in GraphiQL, everything worked, and I thought I could simply copy-paste it into my JavaScript code and prepare a pull request. Later I found out a bug where the sort was not actually working as expected.

With this pull requests clients will know much sooner that there is something wrong with their query, and be able to fix it saving some time.

Cheers
Bruno

@kinow kinow changed the title Raise an error when invalid sort keys are provided by the client Raise an error when invalid sort keys are provided by a GraphQL client Mar 8, 2021
tests/unit/network/test_schema.py Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member

Change log conflict, and:

./tests/unit/network/test_schema.py:27:15: W291 trailing whitespace

@kinow
Copy link
Member Author

kinow commented Mar 10, 2021

Fixed changelog conflict, and extra space. Thanks @hjoliver !

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

👍

@hjoliver hjoliver merged commit a891703 into cylc:master Mar 10, 2021
@kinow kinow deleted the warn-invalid-sort-keys branch March 10, 2021 23:05
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.

Warn if the key used in the "sort" argument is not in the scheme
3 participants