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.

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

Replace python-ldap with openldap in conda env #1524

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Replace python-ldap with openldap in conda env #1524

wants to merge 1 commit into from

Conversation

fabcor-maxiv
Copy link
Contributor

On PyPI, python-ldap is distributed as an sdist only, not wheel. But it still requires a compilation step before installation.

Some dependencies installed with conda are rightfully ignored by Poetry. Even when python-ldap is pre-installed by conda, Poetry would still need to reinstall it which implies compilation. Indeed there is an issue in how conda packages are created and how they are installed:

So it does not make much sense to let conda install python-ldap. Instead we can instruct conda to install openldap only, and pip and Poetry should be able to compile python-ldap.

GitHub: #1510
GitHub: mxcube/mxcubecore#849 (comment)
GitHub: conda-forge/python-ldap-feedstock#28
GitHub: mxcube/mxcubecore#851

@fabcor-maxiv
Replace python-ldap with openldap in conda env
c98a431 
On PyPI, `python-ldap` is distributed as an *sdist* only, not *wheel*.
But it still requires a compilation step before installation.

Some dependencies installed with conda are rightfully ignored by Poetry.
Even when `python-ldap` is pre-installed by conda,
Poetry would still need to reinstall it which implies compilation.
Indeed there is an issue in how conda packages are created
and how they are installed:
* <python-poetry/poetry#6408 (comment)>
* <conda-forge/python-ldap-feedstock#28>

So it does not make much sense to let conda install `python-ldap`.
Instead we can instruct conda to install `openldap` only,
and pip and Poetry should be able to compile `python-ldap`.

GitHub: mxcube#1510
GitHub: mxcube/mxcubecore#849 (comment)
GitHub: conda-forge/python-ldap-feedstock#28
GitHub: mxcube/mxcubecore#851
@fabcor-maxiv
Copy link
Contributor Author

fabcor-maxiv commented Jan 14, 2025 •
edited
Loading

@axelboc if you have time and opportunity, please test if this approach solves the issue you reported in #1510.

Honestly I do not know if this approach is suitable for everyone.

It feels like one of those things where whatever we change (good or bad), it will break someone's workflow. : D

But anyway, I feel like something has to be done. For example, when running poetry install in a fresh conda environment created with the YAML file, I see a bunch of "Updating" and "Downgrading" entries. Those are all cases where Poetry overwrites a conda package, and these all look like red flags to me:

  - Downgrading certifi (2024.12.14 /home/conda/feedstock_root/build_artifacts/certifi_1734380492396/work/certifi -> 2024.8.30)
  - Updating charset-normalizer (3.4.0 /home/conda/feedstock_root/build_artifacts/charset-normalizer_1733218092148/work -> 3.4.0)
  - Updating idna (3.10 /home/conda/feedstock_root/build_artifacts/idna_1733211830134/work -> 3.10)
  - Downgrading numpy (2.2.0 /home/conda/feedstock_root/build_artifacts/numpy_1733688208563/work/dist/numpy-2.2.0-cp311-cp311-linux_x86_64.whl -> 1.24.4)
  - Downgrading setuptools (75.6.0 -> 75.3.0)
  - Updating urllib3 (2.2.3 /home/conda/feedstock_root/build_artifacts/urllib3_1733206856906/work -> 2.2.3)
  - Downgrading packaging (24.2 /home/conda/feedstock_root/build_artifacts/packaging_1733203243479/work -> 24.1)
  - Updating pycparser (2.22 /home/conda/feedstock_root/build_artifacts/bld/rattler-build_pycparser_1733195786/work -> 2.22)
  - Updating requests (2.32.3 /home/conda/feedstock_root/build_artifacts/requests_1733217035951/work -> 2.32.3)
  - Updating cffi (1.17.1 /home/conda/feedstock_root/build_artifacts/cffi_1725560564262/work -> 1.17.1)
  - Updating distlib (0.3.9 /home/conda/feedstock_root/build_artifacts/distlib_1733238395481/work -> 0.3.9)
  - Updating filelock (3.16.1 /home/conda/feedstock_root/build_artifacts/filelock_1733240801289/work -> 3.16.1)
  - Updating platformdirs (4.3.6 /home/conda/feedstock_root/build_artifacts/platformdirs_1733232627818/work -> 4.3.6)
  - Downgrading tomli (2.2.1 /home/conda/feedstock_root/build_artifacts/tomli_1733256695513/work -> 2.0.2)
  - Updating colorama (0.4.6 /home/conda/feedstock_root/build_artifacts/colorama_1733218098505/work -> 0.4.6)
  - Downgrading cryptography (44.0.0 /home/conda/feedstock_root/build_artifacts/cryptography-split_1732745928243/work -> 43.0.3)
  - Updating importlib-resources (6.4.5 /home/conda/feedstock_root/build_artifacts/importlib_resources_1733231327252/work -> 6.4.5)
  - Downgrading virtualenv (20.28.0 /home/conda/feedstock_root/build_artifacts/virtualenv_1732609431896/work -> 20.27.1)

@axelboc
Copy link
Collaborator

axelboc commented Jan 14, 2025

Worked like a charm!!

micromamba env create --file conda-environment.yml
micromamba activate mxcubeweb
poetry install
mxcubeweb-server -r $(pwd)/demo/ --static-folder $(pwd)/ui/build/ -L debug

micromamba activate mxcubeweb
pnpm --prefix ui install
pnpm --prefix ui start

The UI shows up and I can log in. Like a charm, as I said! 😍

@axelboc
Copy link
Collaborator

axelboc commented Jan 14, 2025 •
edited
Loading

Of course, I haven't tested in a fully fresh Ubuntu 24 install.

  • I think the ffmpeg issue has been resolved/documented.
  • The hopefully ESRF-specific timezone misconfiguration is most likely fixed as well.
  • And if I understand correctly, build-essential should no longer be needed? I've just uninstalled it from my machine and recreated the environment and it seems to work!

@fabcor-maxiv
Copy link
Contributor Author

  • And if I understand correctly, build-essential should no longer be needed?

This is the part I am not entirely sure about. Maybe I should try in an empty container, where all this stuff is absent, no compiler, no Python dev headers.

@fabcor-maxiv
Copy link
Contributor Author

  • And if I understand correctly, build-essential should no longer be needed?

This is the part I am not entirely sure about. Maybe I should try in an empty container, where all this stuff is absent, no compiler, no Python dev headers.

Not so surprisingly, a compiler is required. Seems to me like gcc is enough (i.e. not the whole of build-essential). I do not know if it is safe to assume that everyone has a compiler installed, or if we should add it to the list of "Compile-time dependencies" in conda-environment.yaml.

@marcus-oscarsson
Copy link
Member

Thanks, very nice :) I think its rather safe to assume that there is a compiler installed, I think we can address that issue if it arises.

@marcus-oscarsson
Copy link
Member

I think conda-environment used to contain openldap but it was removed in some cleanup

@fabcor-maxiv
Copy link
Contributor Author

I think conda-environment used to contain openldap but it was removed in some cleanup

I have not checked yet, but I would not be surprised if I was the one to remove it. : D

I will try to check this and further down the history of this file, maybe there are some valuable lessons to learn from the past.

@fabcor-maxiv
Copy link
Contributor Author

  1. 2017-11-28:
  2. 2021-01-07
  3. 2022-12-19
  4. 2023-02-20
  5. 2024-06-04

Of note, the discusssion #954. To some extent, Benjamin is right, but because of the following issue (that I already mentioned many times), this all falls apart and Poetry will rightfully want to compile python-ldap anyway, even if conda already installed the correct version:

I will discuss with @beenje, maybe I can convince him to pair up for an adventure and fix things on conda's side. : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fabcor-maxiv @axelboc @marcus-oscarsson