chore: pin python package dependencies

[SMoraisAnsys]

Changes to address #985.

[MaxJPRey]

Thanks for taking care of that @SMoraisAnsys

[jorgepiloto]

Left some questions. The only thing I am concerned is that this causes some issues when using the actions since we are now imposing a specific version for some packages.

[SMoraisAnsys]

Left some questions. The only thing I am concerned is that this causes some issues when using the actions since we are now imposing a specific version for some packages.

I'm not sure we'll end up with any issue tbh. AFAIK the packages are only pinned for actions were we already had a working venv which was created from scratch and should have been modified after package installation.
Edit: Just found that at least the pytest action can be an issue when one wants to use it with randomize. In that case pytest-randomly is being installed before running the tests. Note that I also discovered a bug on the fly as the venv isn't activated before installing the package. Since this action is compatible with uv, pip and poetry, I'm not sure how we should handle the addition of pytest-randomly...

Guess it's a good candidate to leverage the pre release feature ? :D

[SMoraisAnsys]

I've updated the code of the pytest action as follow

    - uses: ansys/actions/_logging@main
      if: inputs.randomize == 'true'
      with:
        level: "WARNING"
        message: >
          Installing pytest-randomly on top of the test dependencies as requested.
          This might lead to dependency conflicts. Consider adding pytest-randomly
          to your test dependencies instead.

    - name: "Randomize the order of the tests"
      shell: bash
      if: inputs.randomize == 'true'
      env:
        ACTION_PATH: ${{ github.action_path }}
      run: |
        if [[ "$BUILD_BACKEND" == 'poetry' ]]; then
          poetry run pip install pytest-randomly==3.16.0
        if [[ "$BUILD_BACKEND" == 'uv' ]]; then
          uv pip install pytest-randomly==3.16.0
        else
          python -m pip install pytest-randomly==3.16.0
        fi

[SMoraisAnsys]

@jorgepiloto @moe-ad It's been more than a month that this PR is open, should it be closed ? If not I can refresh my mind on its content cause I've lost track of the associated changes.

[jorgepiloto]

Last thing before merging this is to update the dependabot.yml so that it is aware about these new requirements.txt files.

[SMoraisAnsys]

Last thing before merging this is to update the dependabot.yml so that it is aware about these new requirements.txt files.

Let's discuss about those changes during tomorrow's meeting to see if we are missing something.

[SMoraisAnsys]

Following today's discussion, I'm switching to draft mode. Here are the available options to be explored (by order of personal taste):

1. Use uv lock file (might require a TOML file to be around but should be dependabot comaptible and allow transient updates)
2. Use poetry lock file - same as uv BUT uv is the default manager to install packages in our actions.
3. Use pip freeze but this would require an additional job to handle the addition of transient dependency when dependabot opens a PR (say that dependabot bump A whose new version requires B then we would need to catch this update which wouldn't be caught by dependabot alone).

[moe-ad]

Tests:

• https://github.com/ansys-internal/test-repo-moe/pull/20
• https://github.com/ansys-internal/test-repo-moe/pull/21
• https://github.com/ansys-internal/test-repo-moe/pull/19
• https://github.com/ansys-internal/test-repo-moe/pull/24

[moe-ad]

Important things to note:

• Each action's dependencies are grouped into distinct dependency groups in the added pyproject.toml file.
• towncrier.toml has been removed in favour of specifying the configuration in the pyproject.toml file.
• Dependabot's support for uv is not perfect yet. Among other current limitations, the uv.lock file it generates is sometimes incorrect. Start from this issue for more information about that.
• The workaround is to have a fix job that targets dependabot's uv update branches
• The uv.lock file is the single source of truth, and the requirements files are being exported automatically from it. This process ensures that transient dependencies are properly caught.
• The fix job is also taken advantage of to automatically sync the requirements files with the uv.lock file.

[moe-ad]

Extra things we should explore (Sebastien's feedback):

• We should consider using to major.minor instead of major.minor.patch for the bounds in the pyproject.toml file.
• Have an explicit option to switch off the installation of pinned versions. Make the installation of pinned dependencies an opt-in option for the next minor release and the default behavior for the next major release. That way, it will always to possible to revert to legacy behavior (if something is breaking or just preferred).
• Explore if there is a way to make poetry consume the exported requirements files as well.

[RobPasMue]

IMO...

We should consider pinning to major.minor instead of major.minor.patch.

Pinning down means pinning down... so, all the way down to patch is the right approach I believe.

Have an explicit option to switch off the installation of pinned versions. Make the installation of pinned dependencies an opt-in option for the next minor release and the default behavior for the next major release. That way, it will always to possible to revert to legacy behavior (if something is breaking or just preferred).

Shouldn't things be more under control with the pinned approach? And if a user decides not to use the pinned versions then.. does that mean we let pip decide the version in many ocassions?

Explore if there is a way to make poetry consume the exported requirements files as well.

Unix --> poetry add $(cat requirements.txt)
Windows --> Get-Content requirements.txt | ForEach-Object { poetry add $_ }

And then simply use poetry install.

[jorgepiloto]

@moe-ad and I just had a quick discussion. We agreed on delaying the implementation of this feature until the next major. The reason is that this seems a bit hacky and unstable yet. Thus, making a minor release could cause a silent failure for projects not using pinned SHA versions.

[SMoraisAnsys]

IMO...

We should consider pinning to major.minor instead of major.minor.patch.

Pinning down means pinning down... so, all the way down to patch is the right approach I believe.

@moe-ad When we discussed, I mentioned using "major.minor instead of major.minor.patch" for the bounds in the pyproject.toml file. The idea behind that was to reduce the CI load, which is curretly heavy, and not to pin to "major.minor". Either the sentence is wrongly formulated or something needs to be clarified.

Have an explicit option to switch off the installation of pinned versions. Make the installation of pinned dependencies an opt-in option for the next minor release and the default behavior for the next major release. That way, it will always to possible to revert to legacy behavior (if something is breaking or just preferred).

Shouldn't things be more under control with the pinned approach? And if a user decides not to use the pinned versions then.. does that mean we let pip decide the version in many ocassions?

I think we should still leave people with the ability to leverage the pinned version or not. Defaulting to pinned version is fine but if a conflict of dependency resolution / compatibility arise, having the ability to disable the pinned version temporarily would allow workflows to run while the issue is investigated. If something needs to be changed on the action side, it would require a new release to be performed :/

Explore if there is a way to make poetry consume the exported requirements files as well.

Unix --> poetry add $(cat requirements.txt) Windows --> Get-Content requirements.txt | ForEach-Object { poetry add $_ }

And then simply use poetry install.

Yeah, that could be a simple way of handling it.

[moe-ad]

@moe-ad When we discussed, I mentioned using "major.minor instead of major.minor.patch" for the bounds in the pyproject.toml file. The idea behind that was to reduce the CI load, which is curretly heavy, and not to pin to "major.minor". Either the sentence is wrongly formulated or something needs to be clarified.

My bad. I have updated the sentence.

[SMoraisAnsys]

Seems like the problem of uv-lock file update with dependabot has been (re-re-re?)fixed dependabot/dependabot-core#10478 (comment)