DEV - Clean and update dependencies

[trallard]

Thanks to @pavithraes we realised that now runtime dependencies for conda-store(-server) are not up to date nor included in distributed versions of the package.

This is in part due to having bloated environment.yaml files that contain both dev and runtime dependencies (that should only de specified in the pyproject.toml). Since these are both used in local development and CI missing dependencies have gone unnoticed. This PR aims to clean the dependencies and enviroments.

Description

This pull request:

• Adds conda-store(-server) runtime dependencies to the corresponding pyproject.toml
• Keeps dev only dependencies in environment.yaml
• Bumps GH actions and pre-commit versions

Pull request checklist

• Did you test this change locally?
• Did you update the documentation (if required)?
• Did you add/update relevant tests for this change (if required)?

Additional information

How to test

[netlify]

✅ Deploy Preview for kaleidoscopic-dango-0cf31d canceled.

Name	Link
🔨 Latest commit	48ec413
🔍 Latest deploy log	https://app.netlify.com/sites/kaleidoscopic-dango-0cf31d/deploys/65fb1ad491da1900084dd14d

[netlify]

✅ Deploy Preview for kaleidoscopic-dango-0cf31d canceled.

Name	Link
🔨 Latest commit	16911ad
🔍 Latest deploy log	https://app.netlify.com/sites/kaleidoscopic-dango-0cf31d/deploys/6601b3bf836dc800085cc60d

[pavithraes]

Thank you, this looks great!

I took a look at the docs updates. I also tried local+dev install conda-store against this branch, and all seemed good.

The suggestions are typo-fixes, so I'll approve.

[nkaretnikov]

@trallard Thanks for your contribution! I think we need to rethink the approach here.

To summarize:

• This PR makes too many unrelated changes: whitespace, documentation, version bumps, formatting, which makes it harder to review
• I suggest keeping only absolutely essential things here (for the purposes of adding missing dependencies to the pyproject file): env yaml files, workflows, pyproject update, and splitting out everything else into one or more self-contained PRs
• Moreover, I think the current approach needs rethinking altogether. The initial goal of this PR is to add missing dependencies to the pyproject file. But as is, it tries to change the existing testing infra, which relies heavily on env yaml files in several places, which is also what devs use to test and run the code locally (as opposed to using hatch to build via pyproject)
• Changing too many things at once is error-prone. So I suggest this instead: add missing dependencies to the pyproject file as needed, add a completely separate workflow that would use the pyproject file and would run unit tests against conda-store installed via hatch, keep everything else as is. I understand this is not ideal because of duplication. But this way we keep the good known baseline around until we have a working solution with hatch. After a while, once we're comfortable with new testing infra, we can retire the old one.

I'm happy to have an offline chat about this or take over parts of this PR if needed.

[nkaretnikov]

Why is this change (and other like it) necessary now?

[trallard]

Right now everything is run with bare pytest.... or similar and we have multiple installations of Python as is. Adding the (I)m ensures we are using the actual executable we intend to.
I might end up removing this once all the dependencies and workflows are aligned but without isolating in the meantime it is hard to tell what is being called

[nkaretnikov]

This file is used to create the docker image, which is used in integration tests, see conda-store-server/Dockerfile:

COPY environment.yaml /opt/conda-store-server/environment.yaml

RUN mamba env create -f /opt/conda-store-server/environment.yaml && \
	conda clean  --force-pkgs-dirs

I know for sure that constructor is needed, so these changes need to be reviewed and dependencies removed carefully one by one.

[trallard]

constructor is a runtime dependency of conda-store itself so it should be already installed.

We do not need to keep double installing dependencies - runtime and dev.
This is the whole purpose of the PR separating runtime and dev concerns as currently they are all mushed up (runtime dependencies are in dev so when they were not added to the pyproject.toml it was always obscured as they were installed with the dev env file)

[nkaretnikov]

Same here. The dependencies need to be reviewed carefully. For instance, this file is used by integration tests to run the test code. Currently, the integration tests don't run at all - they fail during the collect step because a different version of pydantic is being used (instead of the <2 version specified above).

From the test output:

==================================== ERRORS ====================================
______________________ ERROR collecting tests/test_api.py ______________________
../tests/test_api.py:25: in <module>
    from conda_store_server import schema
conda_store_server/schema.py:50: in <module>
    class AuthenticationToken(BaseModel):
conda_store_server/schema.py:55: in AuthenticationToken
    role_bindings: Dict[constr(regex=ARN_ALLOWED), List[str]] = {}
E   TypeError: constr() got an unexpected keyword argument 'regex'
=========================== short test summary info ============================
ERROR ../tests/test_api.py - TypeError: constr() got an unexpected keyword argument 'regex'
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 0.34s ===============================

The workflow code:

  integration-test-conda-store-server:
...
        with:
          environment-file: conda-store-server/environment-dev.yaml

I suspect there will be other failures once you add pydantic back because the tests import conda_store_server.

[nkaretnikov]

constructor is a runtime dependency and needs to be in the environment for it to work, which is why you're seeing these unit test failures:

tests/test_actions.py::test_generate_constructor_installer

[trallard]

Exactly is a runtime dependency not a dev dependency. So it does not need to be in the environment.

[trallard]

This PR makes too many unrelated changes: whitespace, documentation, version bumps, formatting, which makes it harder to review

Documentation is not unrelated -> addresses changes to the workflow and adds missing installation instructions.

Whitespaces / formatting -> this sneaked in through pre-commit.ci before I could make the updates to the config 🤷🏽 I can revert this as it is the only "unrelated" change

I suggest keeping only absolutely essential things here (for the purposes of adding missing dependencies to the pyproject file): env yaml files, workflows, pyproject update, and splitting out everything else into one or more self-contained PRs

Everything is mushed together atm; I can split it into smaller PRs, which is not a problem, but all are ultimately dependencies.

Moreover, I think the current approach needs rethinking altogether. The initial goal of this PR is to add missing dependencies to the pyproject file. But as is, it tries to change the existing testing infra, which relies heavily on env yaml files in several places, which is also what devs use to test and run the code locally (as opposed to using hatch to build via pyproject)

It started by "trying to add" missing dependencies, but without adequate approaches to how these are added, tested, and used, it does not address the root cause - which is all dependencies are treated the same, and we rely solely on yaml environment files for all the needs. This does not change anything from using yaml env files to hatch at all.
Regardless of whether you use environment-dev.yaml to set your local environment, you still need to do pip install—e . This PR keeps the use of the yaml environment files but separates dev dependencies from conda-store and conda-store-server, the underlying workflows and processes remain unchanged.
For contributors/maintainers at most this PR would mean they need to update their local development environment, but there are no material changes to anything else.

Changing too many things at once is error-prone. So I suggest this instead: add missing dependencies to the pyproject file as needed, add a completely separate workflow that would use the pyproject file and would run unit tests against conda-store installed via hatch, keep everything else as is. I understand this is not ideal because of duplication. But this way we keep the good known baseline around until we have a working solution with hatch. After a while, once we're comfortable with new testing infra, we can retire the old one.

There is no new and old testing infra; it is just cleaning where dependencies are declared and sourced.

[trallard]

Will close this as all the additions here have been split across other PRs