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

actions: provision python from conda-forge #6655

Merged
merged 7 commits into from
Mar 11, 2025

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Mar 5, 2025

  • Provision Python , Bash and other deps from conda-forge.
  • Divorce from the OS environment and give us more flexibility in the software versions we utilise.
  • Reduce usage of apt get (to at) and remove usage brew install completely.
  • Fix SQL statements in the functional tests to comply with changes in sqlite3 3.46.0+ (closes sqlite3: migrate [tests] to 3.46.0+ #6630)
  • Fix a functional test (wasn't failing before for unknown reasons, but the fix makes sense)
  • Install subversion to ensure svn tests aren't skipped.
  • Fix installation of the mock mail command to ensure mail tests aren't skipped.

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 if present).
  • Tests are included (or explain why tests are not needed).
  • Changelog 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, PR should be raised against the relevant ?.?.x branch.

@oliver-sanders oliver-sanders added this to the 8.4.2 milestone Mar 5, 2025
@oliver-sanders oliver-sanders self-assigned this Mar 5, 2025
@oliver-sanders oliver-sanders force-pushed the actions.micromamba branch 3 times, most recently from a717fa0 to c3e23d2 Compare March 6, 2025 09:50
@MetRonnie MetRonnie added the infrastructure GH Actions, Codecov etc. label Mar 6, 2025
Comment on lines 82 to +90
- name: 'macos 1/5'
os: 'macos-latest'
python-version: '3.9'
python-version: '3.8' # oldest available
test-base: 'tests/f'
chunk: '1/5'
platform: '_local_background*'
- name: 'macos 2/5'
os: 'macos-latest'
python-version: '3.9'
python-version: '3' # newest available
Copy link
Member

Choose a reason for hiding this comment

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

The original intention was to run the 1/4 chunk in MacOS on the oldest Python version. (However at some point this was split into 1/5 and 2/5 for faster completion.)

What is the intention behind running different chunks in different Python versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought we should probably have a test for latest Python on Mac OS as a canary. Happy to revert (unrelated really).

Comment on lines +160 to +163
sudo apt-get update
sudo apt-get install -y at
Copy link
Member Author

Choose a reason for hiding this comment

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

at is the one thing we need to get from the OS package manager.

@@ -12,6 +12,10 @@ concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

defaults:
run:
shell: bash -c "exec $CONDA_PREFIX/bin/bash -elo pipefail {0}"
Copy link
Member Author

Choose a reason for hiding this comment

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

Setting -eo pipefail for error checking.

@oliver-sanders oliver-sanders requested a review from MetRonnie March 6, 2025 13:45
@MetRonnie

This comment was marked as resolved.

@MetRonnie

This comment was marked as resolved.

* Install system dependencies from conda-forge rather than relying on
  the OS package manager.
* This divorces us from the system environment / underlying OS image,
  isolating us from changes to it.
* This gives us more flexibility in the versions of the systems we
  install (e.g. no need to change OS image to install an older/newer
  version of Python).
* Closes cylc#6630
* Fixes SQL statements in functional tests to recent changes in sqlite3.
* Remove trailing newline character from SCP command.
@oliver-sanders

This comment was marked as resolved.

@oliver-sanders

This comment was marked as resolved.

* We were adding the `mail` command to `$PATH` in the `~/.bashrc` file,
  however, this file returns early when not run in interactive mode
  (which is always in a CI environment).
* Moved this into the `~/.bash_profile` file.
@oliver-sanders
Copy link
Member Author

Ok, comments addressed:

  • Fixed the mocked mail command (I think this was broken before). The mail tests are now running e.g..
  • Installed the svn command (this was broken by GH image changes).
  • Fixed an SQL statement and updated the test to ensure the failure of this operation fails the test.

Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Nice. Got one comment suggestion that I would rather have merged but not a blocker

@oliver-sanders
Copy link
Member Author

Done.

@MetRonnie MetRonnie merged commit c252734 into cylc:8.4.x Mar 11, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure GH Actions, Codecov etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants