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.

Sign up for GitHub

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

Jump to bottom

id: preserve selectors for "task" and "workflow" properties #5188

Merged
merged 3 commits into from
Oct 13, 2022
Merged

id: preserve selectors for "task" and "workflow" properties #5188

merged 3 commits into from
Oct 13, 2022

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Oct 11, 2022 •
edited
Loading

  • Selectors were not correctly preserved for the task and workflow properties of Tokens objects.
  • This effectively stripped cycle/task selectors from IDs passed to cylc trigger.
  • Closes Task ID filtering broken #5186

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).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PRs raised to both master and the relevant maintenance branch.

@oliver-sanders oliver-sanders self-assigned this Oct 11, 2022
@oliver-sanders oliver-sanders added the bug Something is wrong :( label Oct 11, 2022
@oliver-sanders oliver-sanders added this to the cylc-8.0.3 milestone Oct 11, 2022
@oliver-sanders oliver-sanders modified the milestones: cylc-8.0.3, cylc-8.0.4 Oct 12, 2022
@MetRonnie MetRonnie changed the title id: perserve selectors for "task" and "workflow" properties id: preserve selectors for "task" and "workflow" properties Oct 12, 2022
hjoliver
hjoliver approved these changes Oct 12, 2022
View reviewed changes
Copy link
Member

@hjoliver hjoliver left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM hang on ...

hjoliver
hjoliver reviewed Oct 12, 2022
View reviewed changes
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.

Doesn't fix the original reported problem...

hjoliver
hjoliver requested changes Oct 13, 2022
View reviewed changes
Copy link
Member

@hjoliver hjoliver left a comment •
edited
Loading

Choose a reason for hiding this comment

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

OK, to close the issue we need to detokenise IDs with selectors=True in the CLI scripts. I'll post a PR to your branch.

I'd like to get this into 8.0.3 - consider it approved if my side-PR is good.

This was referenced Oct 13, 2022
Merged
Merged
oliver-sanders and others added 2 commits October 13, 2022 16:06
@oliver-sanders
id: perserve selectors for "task" and "workflow" properties
3789594 
* Selectors were not correctly preserved for the `task` and
  `workflow` properties of `Tokens` objects.
* This effectively stripped cycle/task selectors from
  IDs passed to `cylc trigger`.
* Closes cylc#5186
@hjoliver @oliver-sanders
CLI: detokenise with selectors. (#57)
3f9a333 
uid: include selectors in detokenised IDs where required

* Add a new interface for retrieving the string ID including selectors from `Tokens` objects.
* Use this for CLI scripts which utilise selectors (e.g. `kill`).
* Fixes a bug where selectors provided on the CLI were silently stripped before being passed through GraphQL.
@oliver-sanders
Copy link
Member Author

(deconflicted)

hjoliver
hjoliver approved these changes Oct 13, 2022
View reviewed changes
@hjoliver hjoliver merged commit a149776 into cylc:8.0.x Oct 13, 2022
@hjoliver hjoliver mentioned this pull request Oct 13, 2022
Closed
@oliver-sanders oliver-sanders deleted the 5186 branch October 14, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjoliver hjoliver hjoliver approved these changes

@MetRonnie MetRonnie MetRonnie approved these changes

Assignees

@oliver-sanders oliver-sanders

Labels
bug Something is wrong :(
Projects
None yet
Milestone
cylc-8.0.3
Development

Successfully merging this pull request may close these issues.
3 participants
@oliver-sanders @hjoliver @MetRonnie