Add workflow to test conda packages #1935
Conversation
WalkthroughThe changes introduce a new GitHub Actions workflow for building and testing a Conda package across multiple operating systems. Additionally, the existing PyPI workflow is updated to reflect a more specific focus on PyPI tasks, with minor formatting improvements. These modifications aim to streamline the automation processes related to package building and testing. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Conda Environment
participant Test Environment
Developer->>GitHub Actions: Push changes
GitHub Actions->>Conda Environment: Set up environment
GitHub Actions->>Conda Environment: Build Conda package
Conda Environment-->>GitHub Actions: Upload artifacts
GitHub Actions->>Test Environment: Set up testing environment
Test Environment-->>GitHub Actions: Run tests
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
.github/workflows/build_conda_ci.yml (1)
16-19: Use of environment variables.Defining environment variables at the workflow level is a good practice for reusability and maintaining consistency across jobs. However, the specific purpose of
RUN_IDshould be documented for clarity.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/build_conda_ci.yml (1 hunks)
- .github/workflows/build_pypi_ci.yml (5 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/build_pypi_ci.yml
Additional context used
actionlint
.github/workflows/build_conda_ci.yml
139-139: shellcheck reported issue in this script: SC1073:error:1:1: Couldn't parse this if expression. Fix to allow more checks
(shellcheck)
139-139: shellcheck reported issue in this script: SC1050:error:1:36: Expected 'then'
(shellcheck)
139-139: shellcheck reported issue in this script: SC1072:error:1:37: Unexpected . Fix any mentioned problems and try again
(shellcheck)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
Additional comments not posted (8)
.github/workflows/build_conda_ci.yml (8)
1-3: Clear and concise workflow description.The initial comments and the name of the workflow are clear and provide a good overview of the workflow's purpose.
5-14: Appropriate triggers for the workflow.The workflow is triggered by changes to relevant files, ensuring that any modifications that could affect the Conda build process are tested. This is a good practice as it helps in catching issues early in the development cycle.
20-42: Well-structured job definition with matrix strategy.The matrix strategy is effectively used to run builds across different operating systems. The inclusion of specific configurations for each OS via the
includekeyword is a good practice, ensuring that each environment is correctly set up.
43-67: Detailed setup steps for the build environment.The steps for checking out the code, setting up Miniconda, and printing the build environment info are well-defined. Using conditions to control the execution of these steps based on the
RUN_BUILD_JOBenvironment variable is a good practice to optimize workflow runs.
68-83: Conditional build steps for different OS.The separation of build steps for Windows and other OS is necessary due to different shell environments and potential build differences. This is well-handled here.
84-93: Efficient handling of build artifacts.Uploading the build artifacts with a retention policy and allowing overwrites is a good practice for CI workflows, especially when subsequent jobs, like tests, depend on these artifacts.
94-126: Comprehensive testing setup.The testing job is well-structured with its own matrix strategy and includes specific configurations for different OS. The use of conditional steps based on the OS for setting up the test environment is a good practice.
127-194: Detailed steps for testing the package.The steps for checking out the repository, setting up Python, downloading the build artifact, and setting up Miniconda are well-defined. The final test step is particularly well-detailed, ensuring that the package is correctly installed and tested in a new Conda environment.
Tools
actionlint
139-139: shellcheck reported issue in this script: SC1073:error:1:1: Couldn't parse this if expression. Fix to allow more checks
(shellcheck)
139-139: shellcheck reported issue in this script: SC1050:error:1:36: Expected 'then'
(shellcheck)
139-139: shellcheck reported issue in this script: SC1072:error:1:37: Unexpected . Fix any mentioned problems and try again
(shellcheck)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
instead of misleading "Use current run"
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/build_conda_ci.yml (1 hunks)
Additional context used
actionlint
.github/workflows/build_conda_ci.yml
139-139: shellcheck reported issue in this script: SC1073:error:1:1: Couldn't parse this if expression. Fix to allow more checks
(shellcheck)
139-139: shellcheck reported issue in this script: SC1050:error:1:36: Expected 'then'
(shellcheck)
139-139: shellcheck reported issue in this script: SC1072:error:1:37: Unexpected . Fix any mentioned problems and try again
(shellcheck)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
Additional comments not posted (1)
.github/workflows/build_conda_ci.yml (1)
1-194: Comprehensive Review of the New Conda CI WorkflowThis review covers the newly introduced GitHub Actions workflow file
build_conda_ci.yml, which is designed to build and test the SLEAP package using conda across multiple operating systems. The workflow is well-structured and uses a matrix strategy to handle different environments efficiently. Here are some specific observations and suggestions:
Trigger Paths (Lines 7-14): The workflow is triggered by changes to several critical files, including
.conda/meta.yamlandsetup.py. This setup ensures that any significant changes in the package configuration or dependencies will initiate a new build and test cycle, which is a good practice.Environment Variables (Lines 16-18): The use of environment variables such as
RUN_BUILD_JOBandRUN_IDis appropriate for controlling the flow within the workflow and for use across different jobs. However, the hardcodedRUN_IDmight need to be dynamically generated or fetched depending on the context of the workflow execution.Matrix Strategy (Lines 26-41): The matrix strategy is comprehensive, covering different operating systems and special configurations for macOS. This approach is efficient for testing across multiple environments but ensure that the
condarcandconda-folderpaths are correctly set up for each environment.Deprecated Commands (Lines 139-154): As noted in previous comments and static analysis, the
set-outputcommand is deprecated. It's crucial to update these to the new syntax to avoid future deprecation issues. The recommended changes are:- echo "::set-output name=RUN_ID::$env:GITHUB_RUN_ID" + echo "RUN_ID=$env:GITHUB_RUN_ID" >> $GITHUB_OUTPUT - echo "::set-output name=RUN_ID::${{ github.run_id }}" + echo "RUN_ID=${{ github.run_id }}" >> $GITHUB_OUTPUTThese changes should be applied to ensure the workflow remains functional with future updates to GitHub Actions.
Testing and Package Installation (Lines 180-194): The steps to create and activate the conda environment, and to install and test the package are well laid out. However, ensure that the
conda-channelsand the test commands are correctly configured to avoid issues like the one mentioned with the missingimageio_ffmpegdependency.Overall, the workflow file is well-constructed with attention to detail in handling different environments and testing scenarios. Ensure to update deprecated commands and verify the configuration of dependencies to avoid runtime errors during the CI process.
Tools
actionlint
139-139: shellcheck reported issue in this script: SC1073:error:1:1: Couldn't parse this if expression. Fix to allow more checks
(shellcheck)
139-139: shellcheck reported issue in this script: SC1050:error:1:36: Expected 'then'
(shellcheck)
139-139: shellcheck reported issue in this script: SC1072:error:1:37: Unexpected . Fix any mentioned problems and try again
(shellcheck)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
139-139: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
149-149: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
specifying bash shell
instead of "wheel"
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/build_conda_ci.yml (1 hunks)
Additional context used
actionlint
.github/workflows/build_conda_ci.yml
131-131: shellcheck reported issue in this script: SC2086:info:1:39: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (2)
.github/workflows/build_conda_ci.yml (2)
21-85: Review of Build Job: Well-structured and comprehensive.The build job is well-structured with clear steps for setting up the environment, building the package, and uploading it for subsequent use. Using a matrix strategy to test across different operating systems is a good practice. The use of Conda and Miniconda is appropriate for handling Python package dependencies.
Consider documenting each step in the workflow for clarity, especially for new contributors or external collaborators.
86-179: Review of Test Job: Comprehensive testing setup.The test job is well-organized with steps that ensure the environment is correctly set up and the package is tested across different platforms. The use of matrix strategy for testing is consistent with the build job and helps ensure compatibility across environments.
The step to download and use the build artifact is crucial and appears to be correctly implemented. However, ensure that the environment variables and paths are correctly set to avoid issues during the test execution.
Consider adding more detailed logging or debugging information to help diagnose issues if the tests fail.
Tools
actionlint
131-131: shellcheck reported issue in this script: SC2086:info:1:39: Double quote to prevent globbing and word splitting
(shellcheck)
Description
Previously, we added a workflow in
build_ci.ymlwhich builds and installs SLEAP via the PyPI wheel and then runs our unit tests.In this PR, we add another workflow in
build_conda_ci.ymlthat builds and installs SLEAP through the conda package and then runs our unit tests. Also, I renamedbuild_ci.ymltobuild_pypi_ci.yml. I opted not to bundle the conda package build tests with the pypi build tests as they have some non-overlapping triggers (and the conda build is extremely expensive).However, note that both these workflows use artifact-upload/download between build and test jobs which means that we could download the passing build packages from the provided link in action and use those to upload to conda and PyPI (instead of rebuilding in
build.ymlandbuild_manual.yml).Currently, all package tests fail with:
when we attempt to
. A repo search of develop shows that
imageio-ffmpegwas added as a dependency to the environment yamls, but not to the meta yamls that build the conda packages. This PR does not fix this. Instead #1943 fixes this.Types of changes
Does this address any currently open issues?
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements