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 error if there is a relative path in global.cylc[install]source dirs #4887

Merged
merged 6 commits into from
May 24, 2022

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented May 19, 2022

These changes close #4815

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).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Appropriate tests are included (unit)
  • Appropriate change log entry included.
  • No documentation update required.

@MetRonnie MetRonnie added bug Something is wrong :( small labels May 19, 2022
@MetRonnie MetRonnie added this to the cylc-8.0rc4 milestone May 19, 2022
@MetRonnie MetRonnie self-assigned this May 19, 2022
@MetRonnie MetRonnie marked this pull request as draft May 20, 2022 10:22
@MetRonnie MetRonnie removed the small label May 20, 2022
Comment on lines +1507 to +1508
# Flesh out with defaults
self.expand()
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't change the code actually, just makes it clearer - self._set_default_editors() was doing the expansion all this time

if keys:
for key in keys:
try:
cfg = cfg[key]
except (KeyError, TypeError):
if parents in self.manyparents or key in self.get(parents):
Copy link
Member Author

Choose a reason for hiding this comment

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

This self.get() was expanding the config this whole time

@MetRonnie MetRonnie marked this pull request as ready for review May 20, 2022 16:38
@MetRonnie MetRonnie requested review from wxtim and datamel May 20, 2022 16:46
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

Code read. Manually tested installing workflows with various source dir settings. Code read. Workflows are running smoothly for me and the error is raised appropriately.

@wxtim wxtim merged commit 52b5098 into cylc:master May 24, 2022
@MetRonnie MetRonnie deleted the rel-source-dirs branch May 24, 2022 14:16
wxtim added a commit to wxtim/cylc that referenced this pull request May 27, 2022
* 'master' of https://github.com/cylc/cylc: (47 commits)
  Global platform default directives (cylc#4896)
  Raise an error if the user tries to set owner. (cylc#4860)
  Raise error if there is a relative path in `global.cylc[install]source dirs` (cylc#4887)
  Added a noqa tag to a new flake8-simplify case. (cylc#4895)
  Update change log.
  Don't prompt to clean no workflows.
  Bump to next dev version
  Fix task matching bug (cylc#4881)
  Expand comma separated platform names correctly (cylc#4880)
  resolvers: add a couple of comments
  Prepare release 8.0rc3
  Fix change log.
  Add & amalgamate jinja2 error tests
  shellcheck fix
  Tweak show help text.
  Fix wrong context lines for jinja2 error
  Fix missing context lines for jinja2 error
  Improve new func. test.
  Syntax highlighting: don't symlink files now they are extracted using cylc get-resources
  Emacs syntax highlighting: fix filename pattern matching
  ...
@hjoliver hjoliver modified the milestones: cylc-8.0rc4, cylc-8.0.0 Jul 5, 2022
@MetRonnie MetRonnie mentioned this pull request Apr 10, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging this pull request may close these issues.

global.cylc[install]source dirs should not accept relative paths
4 participants