Skip to content

Config: better validation error for conda.environment #10979

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 11, 2024
Merged

Config: better validation error for conda.environment #10979

merged 2 commits into from
Jan 11, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Jan 2, 2024

Screenshot_2024-01-02_16-20-32

Closes #9008

@humitos humitos requested a review from a team as a code owner January 2, 2024 15:20
@humitos humitos requested a review from stsewd January 2, 2024 15:20
@humitos humitos mentioned this pull request Jan 3, 2024
Merged
14 tasks
@ericholscher ericholscher linked an issue Jan 3, 2024 that may be closed by this pull request
Better error when using conda without an environment file #9008
Closed
Base automatically changed from humitos/new-notification-system to main January 4, 2024 10:58
@humitos humitos requested a review from a team as a code owner January 4, 2024 10:58
@humitos humitos requested a review from agjohnson January 4, 2024 10:58
agjohnson
agjohnson approved these changes Jan 4, 2024
View reviewed changes
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

I reviewed 9079b56, assuming this is the only relevant commit to this PR. Everything looks good!

@humitos
Copy link
Member Author

humitos commented Jan 4, 2024

🤷🏼 -- not sure what happened in this PR, but I will need to rebase. The commit you review is the one, yes 👍🏼

@humitos humitos force-pushed the humitos/require-conda-file branch from 9079b56 to cfbe1c5 Compare January 9, 2024 16:37
@humitos humitos merged commit 76cafd0 into main Jan 11, 2024
@humitos humitos deleted the humitos/require-conda-file branch January 11, 2024 15:45
This was referenced Jan 17, 2024
Merged
Closed
@fabcor-maxiv
Copy link

Seems like the error is triggered even when using build.commands. In other words there is an error even if the conda environment is built and used via custom commands only. I am not convinced it should be an error.

For example, this build, with a .readthedocs.yaml file as follows:

version: 2

build:

  os: "ubuntu-22.04"

  tools:
    python: "mambaforge-22.9"

  commands:
    - "mamba env create --file conda-environment-dev.yml"
    - "mamba run --name mxcubecore poetry install --only=docs,main"
    - "mamba run --name mxcubecore poetry run sphinx-build -T -E -b html -c ./docs/ ./docs/source/ ${READTHEDOCS_OUTPUT}/html/"

I can open a separate ticket if you think it is an actual issue that needs to be tracked.

humitos added a commit that referenced this pull request Jan 17, 2024
@humitos
Config: allow missing conda.environment when using build.commands
8b70707 
Solves #10979 (comment)
@humitos humitos mentioned this pull request Jan 17, 2024
Merged
@humitos
Copy link
Member Author

humitos commented Jan 17, 2024

@fabcor-maxiv Thanks. I opened #11040 to solve this issue 👍🏼

humitos added a commit that referenced this pull request Jan 18, 2024
@humitos
Config: allow missing conda.environment when using build.commands (…
7d7ad16 
…#11040)

Solves #10979 (comment)
fabcor-maxiv added a commit to fabcor-maxiv/mxcubecore that referenced this pull request Feb 5, 2024
@fabcor-maxiv
Fix ReadTheDocs configuration
e007555 
Use the `--yes` flag of `mamba env create` in case environment exists.

Skip `poetry run` when running `sphinx-build`
since Poetry installs in the conda environment directly.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
@fabcor-maxiv fabcor-maxiv mentioned this pull request Feb 5, 2024
Merged
fabcor-maxiv added a commit to fabcor-maxiv/mxcubecore that referenced this pull request Feb 5, 2024
@fabcor-maxiv
Fix ReadTheDocs configuration
ec939d8 
Use the `--force` flag of `mamba env create` in case environment exists.

Skip `poetry run` when running `sphinx-build`
since Poetry installs in the conda environment directly.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
fabcor-maxiv added a commit to mxcube/mxcubecore that referenced this pull request Feb 5, 2024
@fabcor-maxiv
Fix ReadTheDocs configuration
0b7e602 
Use the `--force` flag of `mamba env create` in case environment exists.

Skip `poetry run` when running `sphinx-build`
since Poetry installs in the conda environment directly.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
fabcor-maxiv added a commit to fabcor-maxiv/mxcubeweb that referenced this pull request Feb 7, 2024
@fabcor-maxiv
Fix ReadTheDocs configuration
9a7ac16 
Use the `--force` flag of `mamba env create` in case environment exists.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
fabcor-maxiv added a commit to fabcor-maxiv/mxcubeweb that referenced this pull request Feb 7, 2024
@fabcor-maxiv
Fix ReadTheDocs configuration
0cad2f5 
Use the `--force` flag of `mamba env create` in case environment exists.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
@fabcor-maxiv fabcor-maxiv mentioned this pull request Feb 7, 2024
Merged
marcus-oscarsson pushed a commit to mxcube/mxcubeweb that referenced this pull request Feb 7, 2024
@fabcor-maxiv @marcus-oscarsson
Fix ReadTheDocs configuration
e910d4f 
Use the `--force` flag of `mamba env create` in case environment exists.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
HilbertCorsair pushed a commit to HilbertCorsair/mxcubecore_SOLEIL_PX1 that referenced this pull request Feb 23, 2024
@fabcor-maxiv
Fix ReadTheDocs configuration
0a559f3 
Use the `--force` flag of `mamba env create` in case environment exists.

Skip `poetry run` when running `sphinx-build`
since Poetry installs in the conda environment directly.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
walesch-yan pushed a commit to mxcube/mxcubecore that referenced this pull request Jun 17, 2024
@fabcor-maxiv @marcus-oscarsson
Fix ReadTheDocs configuration
44cb1fc 
Use the `--force` flag of `mamba env create` in case environment exists.

Skip `poetry run` when running `sphinx-build`
since Poetry installs in the conda environment directly.

Remove the workaround for the unwarranted error
about "missing `conda.environment`" configuration setting,
because it has been fixed already.
See:
* readthedocs/readthedocs.org#10979 (comment)
* readthedocs/readthedocs.org#11040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agjohnson agjohnson agjohnson approved these changes

@stsewd stsewd stsewd approved these changes

Assignees

@humitos humitos

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Better error when using conda without an environment file
4 participants
@humitos @fabcor-maxiv @agjohnson @stsewd