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

linter: additional rules #4917

Open
8 of 14 tasks
oliver-sanders opened this issue Jun 16, 2022 · 5 comments · Fixed by #5890
Open
8 of 14 tasks

linter: additional rules #4917

oliver-sanders opened this issue Jun 16, 2022 · 5 comments · Fixed by #5890
Assignees
Milestone

Comments

@oliver-sanders
Copy link
Member

oliver-sanders commented Jun 16, 2022

#4900 has gone beyond a basic sed file and is now the basis for a more generic linter 🎉.

Some follow-on ideas:

Editor integration:

(See #5014)

7 to 8 migration rules

This linter could be a highly valuable tool in the 7->8 upgrade. Also include a couple of Python 2->3 pointers.

  • Missing shebang
    • If Jinja2 or EmPy syntax is detected in the suite.rc or flow.cylc file and the first line of the file is not a matching shebang.
    • (rose suite-run used to tolerate missing shebang lines)
  • Module name changes
    • cylc -> cylc.flow
    • rose -> metomi.rose
    • isodatetime -> metomi.isodatetime
    • In Python files from _ import and import _.
    • In .cylc & .rc files from "_" import
  • Tabs in Python files (e.g. Jinja2 filters)
    • Use spaces to indent Python 3 files, mix of tabs and spaces is a syntax error.
  • Zero-prefixed integers in .cylc, .rc files.
    • E.G. 05 rather than 5, previously accepted, now a Jinja2 syntax error.
  • Suicide triggers
    • No longer needed, use optional outputs (for .cylc files only).
  • platform = $(rose host-select)
    • Advise using built-in host selection via configured platforms rather than $(rose host-select).
    • Note there may be legit reasons to use subshells here (platforms does not offer a filter interface yet).
    • But rose host-select is definitely unnecessary.
  • Deprecated env vars (all files in project).
  • Check for deprecated commands (documented in cylc.flow.scripts.cylc.DEAD_ENDS).
  • Parameter simplification
  • Deprecated task and workflow event template variables
  • Use of rose suite-hook (command removed at Rose 2)
  • CYLC_VERSION={{CYLC_VERSION}} (same for ROSE_ and FCM_). lint: Warn users that CYLC_VERSION={{CYLC_VERSION}} is deprecated #5890
  • Using cylc workflow-state in a task-script, should use the workflow_state xtrigger instead
  • :expired output must be optional if used (also :expire, :expire-all and :expire-any), see optional output extension proposal.

Additional style rules and bug bears

  • ...

Pull requests welcome!

@wxtim
Copy link
Member

wxtim commented May 23, 2023

I'm adding some of these to my currently open PR on Cylc Lint, as the introduction of Arbitrary functions makes many much easier.

The ones about checking python files require further thought, since at the moment cylc lint only looks at *.cylc and *.rc.

@wxtim
Copy link
Member

wxtim commented May 23, 2023

@oliver-sanders is there a list of the deprecated env-vars somewhere?

@oliver-sanders
Copy link
Member Author

@oliver-sanders is there a list of the deprecated env-vars somewhere?

Well volunteered!

$ git grep SUITE -- cylc/flow/etc/job.sh
cylc/flow/etc/job.sh:        export CYLC_SUITE_HOST="${CYLC_WORKFLOW_HOST}"
cylc/flow/etc/job.sh:        export CYLC_SUITE_OWNER="${CYLC_WORKFLOW_OWNER}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_SHARE_DIR="${CYLC_WORKFLOW_SHARE_DIR}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_SHARE_PATH="${CYLC_WORKFLOW_SHARE_DIR}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_NAME="${CYLC_WORKFLOW_ID}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_LOG_DIR="${CYLC_WORKFLOW_LOG_DIR}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_INITIAL_CYCLE_POINT="${CYLC_WORKFLOW_INITIAL_CYCLE_POINT}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_INITIAL_CYCLE_TIME="${CYLC_WORKFLOW_INITIAL_CYCLE_POINT}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_FINAL_CYCLE_POINT="${CYLC_WORKFLOW_FINAL_CYCLE_POINT}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_FINAL_CYCLE_TIME="${CYLC_WORKFLOW_FINAL_CYCLE_POINT}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_WORK_DIR="${CYLC_WORKFLOW_WORK_DIR}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_UUID="${CYLC_WORKFLOW_UUID}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_RUN_DIR="${CYLC_WORKFLOW_RUN_DIR}"
cylc/flow/etc/job.sh:    export CYLC_SUITE_DEF_PATH="${CYLC_WORKFLOW_RUN_DIR}"

And also CYLC_SUITE_SRC_DIR which is now obsolete (SRC or SOURCE?):

  • When used with Rose 2019, CYLC_SUITE_SRC_DIR was the same as CYLC_SUITE_RUN_DIR.
  • Otherwise we advise using cylc install to install resources into the run dir rather than using resources from the development working copy in a live workflow.

@wxtim
Copy link
Member

wxtim commented May 25, 2023

I've tried searching on Git, Github, Google, old versions of Rose and Cylc Docs and shared disks locally and I cannot find a reference to:

  • CYLC_SUITE_SRC_DIR
  • CYLC_SUITE_SOURCE_DIR
  • CYLC_SUITE_SOURCE_PATH
  • CYLC_SUITE_SRC_PATH

@oliver-sanders
Copy link
Member Author

oliver-sanders commented May 30, 2023

It was CYLC_SUITE_DEF_PATH not SRC_DIR sorry, it was documented along with the other job environment variables here:

https://cylc.github.io/cylc-doc/7.9.3/html/suite-config.html#task-job-script-variables

(It's the same story for CYLC_SUITE_DEF_PATH_ON_SUITE_HOST)

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 a pull request may close this issue.

2 participants