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.

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

docs: add documentation about virtual environment and requirements.txt #122

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jklare
Copy link

@jklare jklare commented Apr 6, 2023

  • to allow easier installation and establishment of a stable environment, while tracking the required ansible version and additional dependencies (like netaddr), it is useful to have a requirements.txt with all currently working python dependencies in the root of the repository
  • add documentation on how to use the new requirements.txt to install a python3 venv with all current dependencies

Copy link
Member

@SK1Y101 SK1Y101 left a comment

Choose a reason for hiding this comment

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

+1, adding virtual environments for this is a great step towards portability

@jklare jklare mentioned this pull request Apr 12, 2023
Copy link
Contributor

@ElineMaaikedeWeerd ElineMaaikedeWeerd left a comment

Choose a reason for hiding this comment

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

Some minor comments

```bash
python3 -m venv maas-ansible-venv
source maas-ansible-venv/bin/activate
pip install -r requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest python3 -m pip install [stuff] here

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean by [stuff] ? The idea behind this command ist to showcase how to install the full python environment from the requirements.txt file included in this repository.

Copy link
Contributor

Choose a reason for hiding this comment

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

Replacing pip install with python3 -m pip install is what I was suggesting, to avoid issues with multiple pythons/pips/other mess that occasionally occurs on people's systems. Leaving the rest of the line as is.

Copy link
Author

@jklare jklare Apr 14, 2023

Choose a reason for hiding this comment

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

Ah ok, i like the idea to avoid any mess ups on peoples systems :)
I am not sure however if the additional python3 -m pip install is needed here, since the path of pip should be explicitly defined after sourcing (activating) the virtual environment. Are there a specific example where you have seen this fail or to cause issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not specifically in venv (not something I use often)- however using python3 -m pip install syntax is both consistent with use of pip outside of venv and consistent with the docs if I'm not mistaken- hence I prefer it over pip install

@@ -0,0 +1,12 @@
ansible==7.4.0
ansible-core==2.14.4
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe requiring ansible-core separately is unnecessary, as it's already included in and installed with the current version of ansible

Copy link
Author

@jklare jklare Apr 14, 2023

Choose a reason for hiding this comment

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

The requirements.txt was auto generated by running pip freeze after installing the latest available ansible version. There are other ways to generate the requirements.txt (including manual or template based ones (e.g. https://github.com/jazzband/pip-tools)), but i think to get started here it is the easiest to just go with a currently stable environment including all python dependencies.

jklare added 2 commits August 21, 2023 08:57
* to allow easier installation and establishment of a stable
  environment, while tracking the required ansible version and
  additional dependencies (like netaddr), it is useful to have a
  requirements.txt with all currently working python dependencies in the
  root of the repository
* add documentation on how to use the new requirements.txt to install a
  python3 venv with all current dependencies

Signed-off-by: Jan Klare <jan.klare@bisdn.de>
* when using Mac OS, you need to additionally install the python module
  passlib to not run into the error:

  ```
  crypt.crypt not supported on Mac OS X/Darwin, install passlib python module.
  ```

* add passlib as dependency to requirements.txt

Signed-off-by: Jan Klare <jan.klare@bisdn.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants