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

Create python packages requirements.txt file from template #153

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

enricostano
Copy link
Contributor

@enricostano enricostano commented Jun 25, 2023

Right now we blindly copy a text file to generate the requirement.txt file needed to install all the Python packages we need in our projects.

In some case we need to change the content of requirement.txt based on many possibles ways (as many as Ansible variables allow).

An example is when we want to install a package directly from our local development environment file system meanwhile in staging or production we still want those packages downloaded from Pypi.

With this PR we introduce 3 new variables:

  1. odoo_role_odoo_community_packages: packages with Odoo modules that are mere dependencies of our project, we don't develop directly on them
  2. odoo_role_odoo_project_packages: Odoo modules that we develop for a specific project (classic Odoo custom module, etc)

TODO

Usage example

https://git.coopdevs.org/coopdevs/comunitats-energetiques/odoo-ce-inventory/-/merge_requests/61
⚠️ To test this PR with the Som Comunitats inventory you need to upgrade pip package to a version > 20 ⚠️
(this warning doesn't affect how this PR works, is just related to Som Comunitats needs, you can test this PR with other configurations)

tasks/main.yml Outdated Show resolved Hide resolved
@enricostano enricostano force-pushed the feature/python-packages-requirement-template branch from ed8521f to 6761357 Compare July 3, 2023 12:35
@enricostano enricostano force-pushed the feature/python-packages-requirement-template branch from 6761357 to ee6af03 Compare July 3, 2023 13:07
@enricostano enricostano marked this pull request as ready for review July 3, 2023 13:07
@DaniilDigtyar
Copy link

DaniilDigtyar commented Jul 4, 2023

On testing in a local host, I see that energy_communities isn't installed from a local package, so it doesn't apply the local changes.
This is strange, because I don't understand why it installs the package, energy_communities is not in any dependencies setup.

(odoo) odoo@odoo-ce-new:/opt/odoo$ pip freeze | grep energy
odoo14-addon-energy-communities==14.0.1.1.10
-e [email protected]:coopdevs/comunitats-energetiques/odoo-ce.git@fcd020ac0396ad13ccdb1ff889a5391a5b0f648b#egg=odoo14_addon_energy_project&subdirectory=setup/energy_project
-e [email protected]:coopdevs/comunitats-energetiques/odoo-ce.git@fcd020ac0396ad13ccdb1ff889a5391a5b0f648b#egg=odoo14_addon_energy_project_energy_communities&subdirectory=setup/energy_project_energy_communities
-e [email protected]:coopdevs/comunitats-energetiques/odoo-ce.git@fcd020ac0396ad13ccdb1ff889a5391a5b0f648b#egg=odoo14_addon_energy_selfconsumption&subdirectory=setup/energy_selfconsumption
-e [email protected]:coopdevs/comunitats-energetiques/odoo-ce.git@fcd020ac0396ad13ccdb1ff889a5391a5b0f648b#egg=odoo14_addon_energy_selfconsumption_cooperator&subdirectory=setup/energy_selfconsumption_cooperator

@DaniilDigtyar
Copy link

requirements.txt generated

# Ansible managed

python-keycloak==2.9.0
python-slugify==8.0.1
odoo14-addon-account-asset-management==14.0.2.8.0
odoo14-addon-account-banking-mandate==14.0.1.2.0
odoo14-addon-account-banking-pain-base==14.0.1.0.2
odoo14-addon-account-banking-sepa-credit-transfer==14.0.1.2.1
odoo14-addon-account-banking-sepa-direct-debit==14.0.1.3.3
odoo14-addon-account-due-list==14.0.1.1.0
odoo14-addon-account-financial-report==14.0.3.4.1
odoo14-addon-account-fiscal-year==14.0.1.2.0
odoo14-addon-account-payment-mode==14.0.1.1.0
odoo14-addon-account-payment-order==14.0.1.11.0
odoo14-addon-account-payment-partner==14.0.1.7.0
odoo14-addon-account-payment-return==14.0.1.0.5
odoo14-addon-account-payment-return-import==14.0.1.0.3.dev1
odoo14-addon-account-payment-return-import-iso20022==14.0.1.0.1.dev2
odoo14-addon-account-payment-sale==14.0.1.0.1.dev14
odoo14-addon-account-tax-balance==14.0.1.2.6
odoo14-addon-account-menu==14.0.1.1.2.dev5
odoo14-addon-account-multicompany-easy-creation==14.0.1.0.1.dev2
odoo14-addon-base-bank-from-iban==14.0.1.0.2.dev3
odoo14-addon-base-iso3166==14.0.1.0.2.dev1
odoo14-addon-base-location==14.0.1.2.5.dev6
odoo14-addon-base-location-geonames-import==14.0.1.0.3.dev2
odoo14-addon-base-technical-features==14.0.1.1.2.dev1
odoo14-addon-base-user-role==14.0.2.5.1
odoo14-addon-base-user-role-company==14.0.2.0.1
odoo14-addon-base-rest==14.0.4.8.1
odoo14-addon-base-rest-auth-api-key==14.0.1.0.2
odoo14-addon-contract==14.0.2.9.4.dev2
odoo14-addon-contract-payment-mode==14.0.1.1.1
odoo14-addon-contract-variable-quantity==14.0.1.0.1.dev5
odoo14-addon-date-range==14.0.2.1.2.dev3
odoo14-addon-l10n-es-account-asset==14.0.1.0.1.dev6
odoo14-addon-l10n-es-account-statement-import-n43==14.0.1.0.4
odoo14-addon-l10n-es-account-banking-sepa-fsdd==14.0.1.0.1.dev5
odoo14-addon-l10n-es-aeat==14.0.2.4.5
odoo14-addon-l10n-es-aeat-mod111==14.0.2.0.0
odoo14-addon-l10n-es-aeat-mod190==14.0.1.0.1.dev3
odoo14-addon-l10n-es-aeat-mod115==14.0.1.0.1
odoo14-addon-l10n-es-aeat-mod123==14.0.1.0.1.dev5
odoo14-addon-l10n-es-aeat-mod303==14.0.3.5.0
odoo14-addon-l10n-es-aeat-mod390==14.0.3.1.0
odoo14-addon-l10n-es-aeat-mod347==14.0.2.2.0
odoo14-addon-l10n-es-facturae==14.0.2.5.0
odoo14-addon-l10n-es-irnr==14.0.1.0.1
odoo14-addon-l10n-es-mis-report==14.0.1.3.2.dev1
odoo14-addon-l10n-es-partner==14.0.1.0.3.dev1
odoo14-addon-l10n-es-partner-mercantil==14.0.1.0.1.dev4
odoo14-addon-l10n-es-toponyms==14.0.1.0.1.dev4
odoo14-addon-l10n-es-vat-book==14.0.2.4.1.dev1
odoo14-addon-mail-tracking==14.0.2.0.4.dev1
odoo14-addon-mass-editing==14.0.1.1.2
odoo14-addon-mis-builder==14.0.4.1.2.dev2
odoo14-addon-mis-builder-budget==14.0.4.0.2.dev5
odoo14-addon-partner-firstname==14.0.1.1.1.dev4
odoo14-addon-report-py3o==14.0.1.0.3
odoo14-addon-report-xlsx==14.0.1.0.10
odoo14-addon-web-advanced-search==14.0.1.0.2.dev3
odoo14-addon-web-decimal-numpad-dot==14.0.1.0.1.dev2
odoo14-addon-web-environment-ribbon==14.0.1.0.1.dev8
odoo14-addon-web-no-bubble==14.0.1.0.1.dev3
odoo14-addon-web-responsive==14.0.1.2.1.dev12
odoo14-addon-web-timeline==14.0.2.0.1.dev1
odoo14-addon-web-widget-image-download==14.0.1.0.1.dev4
odoo14-addon-auth-oidc==14.0.1.0.3.dev1
odoo14-addon-mass-mailing-list-dynamic==14.0.1.0.1.dev4
odoo14-addon-cooperator==14.0.1.5.2
odoo14-addon-cooperator-website==14.0.1.1.1
git+https://github.com/DaniilDigtyar/[email protected]#egg=odoo14-addon-l10n-es-cooperator&subdirectory=setup/l10n_es_cooperator
git+https://github.com/OCA/[email protected]#egg=odoo14-addon-account-statement-import-txt-xlsx&subdirectory=setup/account_statement_import_txt_xlsx

-e file:///opt/odoo_modules/setup/cooperator_account_payment
-e file:///opt/odoo_modules/setup/cooperator_account_banking_mandate
-e file:///opt/odoo_modules/setup/energy_project
-e file:///opt/odoo_modules/setup/energy_project_energy_communities
-e file:///opt/odoo_modules/setup/energy_selfconsumption
-e file:///opt/odoo_modules/setup/energy_selfconsumption_cooperator
-e file:///opt/odoo_modules/setup/energy_communities

@DaniilDigtyar
Copy link

The same happens with:
odoo14-addon-l10n-es-cooperator==14.0.0.1.1.dev2
odoo14-addon-account-statement-import-txt-xlsx==14.0.3.1.0

Copy link

@DaniilDigtyar DaniilDigtyar left a comment

Choose a reason for hiding this comment

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

LGTM with the new pip version pip>=20

@danypr92 danypr92 requested review from oyale and konykon July 12, 2023 06:41
Copy link
Contributor

@oyale oyale left a comment

Choose a reason for hiding this comment

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

Hey, thanks a lot for your work on this pull request!

I was thinking about rendering different requirements.txt for each environment as an interesting approach, and how it could conflict with our philosophy of uniform stages. Also, I was worrying about if not cleaning up old packages could potentially lead to unforeseen issues.

However, I've been thinking further, and it might be a bit simpler to lean on git a bit more for this.

We could achieve the same by having as many branches as needed. It's a little more streamlined and keeps extra complexity out of our Ansible role. Do you need a different requirements.txt? You've got a new branch!

Is there a use case I'm missing where this git strategy wouldn't work?

Don't get me wrong, your approach is totally valid, and I get why you suggested it. But maybe we can avoid adding more moving parts to our setup by using the tools we already have in place. What do you think?

P.S: I've added some code suggestions regardless.

P.S 2: Having the requirements.txt file rendered in the repository enables us to leverage the entire pip tooling ecosystem. We're currently using it on CI, Odoo Instances, etc...

odoo11-addon-contract-variable-quantity==11.0.1.2.1

In some case you want to deploy different versions of the same module to different hosts.
A typical case is when you have developed a custom module and need to deploy a packaged version of it to production but in local development environment you need to install it es editable.
Copy link
Contributor

@oyale oyale Jul 13, 2023

Choose a reason for hiding this comment

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

Suggested change
A typical case is when you have developed a custom module and need to deploy a packaged version of it to production but in local development environment you need to install it es editable.
A typical case is when you have developed a custom module and need to deploy a packaged version of it to production but in local development environment you need to install it as editable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the "as editable" part since in that use case you'll need to install the package with the -e option, it's just an example, a possible use case.

tasks/main.yml Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@oyale
Copy link
Contributor

oyale commented Jul 13, 2023

I think I'd feel more comfortable sticking with the requirements and adding the ability to uninstall -or delete from requirements.txt at/just after copy time- specific modules on hosts marked for development, through the use of an Ansible task.

This way, we'd keep the advantages of the requirements file, and you could uninstall the pip package of the module you're working on in the dev environment.

Even easier: requirements.txt is not a hard-coded value. It is stored on odoo_role_community_modules_requirements_path which defaults to "{{ inventory_dir }}/../files/requirements.txt"

Just setting it to another requirements file allows you to choose the requirements to install on a host (or any other dark Ansible condition you want) basis.

You could have, e.g. requirements-dev.txt and set odoo_role_community_modules_requirements_path: "{{ inventory_dir }}/../files/requirements-dev.txt" on the host_vars config.

However, haven't we already tackled this? 🤔 It rings a bell that the addon path precedence in odoo.conf allowed the dev environment to pick up your folder instead of the pip package one.

@enricostano
Copy link
Contributor Author

Hi @oyale , thanks for your comments. I'll try to answer in line:

Is there a use case I'm missing where this git strategy wouldn't work?

This is the strategy we first adopted in Som Comunitats and the result is quite a mess since you have to maintain 3 or more "main" branches at the same time.

You could have, e.g. requirements-dev.txt

I've also explored having multiple requirements files (you can also "chain" different files using pip -r option in a file) but it wasn't quite flexible and has some limitation.

I was worrying about if not cleaning up old packages could potentially lead to unforeseen issues.

We will face this issue every time we change the requirements.txt in the host with any of the strategies we're talking about. Even with the current implementation.

P.S 2: Having the requirements.txt file rendered in the repository enables us to leverage the entire pip tooling ecosystem. We're currently using it on CI, Odoo Instances, etc...

The CI argument is valid, although our "inventory" repos have a specific goal of defining variables used by odoo-role or directly odoo-provisioning. Generally speaking generating a file in a host using a template gives you more flexibility compared to copying a file over. Also it allows you to have only one place to look at if you want to debug issues (compared to different branches or tasks that manipulate the file).

As a last point I just wanted to add that the strategy proposed by this PR is what people from Coopdevs, Som IT, Som Comunitats, Som Energia and Som Connexio already agreed upon during the hackday on this specific topic. I'm sorry you couldn't join the session, but how do you feel about giving this approach a try and see if it really doesn't work as a solution? We can always change it and react to any issue it would generate in the future. What do you think?

Many thanks!

@oyale
Copy link
Contributor

oyale commented Aug 1, 2023

Hey, Enrico!

Thank you for taking the time to respond in detail to my comments. I certainly appreciate the thoughts that have gone into the development of this strategy.

Your explanation helped me better understand the big picture; however, I still have some questions. I would like you to expand on a few points, to help us all understand the context and reasoning behind the decisions made:

  1. Different requirements.txt files: You mentioned that you explored the use of different requirements files and found some limitations. Could you explain those limitations?

  2. Maintaining multiple "main" branches: I understand the concerns about the complexity of managing multiple main branches simultaneously. However, I wonder if having to do so might indicate a deeper problem in the approach. Could you help me understand the specific challenges you faced to have to maintain more than three main branches?

  3. Pip tooling replacement: My primary concern is about losing the robust tooling provided by pip's ecosystem. As you've rightly pointed out, these tools are crucial for tasks such as updating package versions, vulnerability analysis, etc. What is your proposed solution for these tasks under the new strategy?

To wrap it up, I believe the loss of the requirements.txt in projects like Som Comunitats signifies a considerable setback for different phases of the process in which we're also involved. I suggest we open a broader discussion around systems' architecture before we adopt such a drastic change. This decision adds complexity and hinders the maintainability of projects we are involved in. Hence, we would need a compelling reason for this significant shift.

As always, I appreciate your efforts to enhance our common software infrastructure and your willingness to engage in these discussions. Looking forward to your thoughts.

Best,

lai.

P.S:

We will face this issue every time we change the requirements.txt in the host with any of the strategies we're talking about. Even with the current implementation.

Absolutely. Maybe it would be nice to add the possibility to reset the venv from playbook.

@oyale
Copy link
Contributor

oyale commented Aug 2, 2023

The CI argument is valid, although our "inventory" repos have a specific goal of defining variables used by odoo-role or directly odoo-provisioning

I'm afraid I'm missing the point here. In fact, our proposal involves using already defined odoo-role variables, setting the odoo_role_community_modules_requirements_path in the inventory repository, rather than merging two new
YAML lists of modules and their versions and rendering them to a brand new requirements.txt at each playbook run.

The inventories act as repositories for various data related to the hosts defined within them, including, as needed, YAML variable files, SSH public keys, CI files, .devenv files, linter/IDE config, and dependency files, don't they? Those that are intended to end in the remote host, such as SSH keys or requirements.txt are processed through Ansible variables to arrive (potentially modified) at their destination and that is where we propose to act.

Generally speaking generating a file in a host using a template gives you more flexibility compared to copying a file over. Also it allows you to have only one place to look at if you want to debug issues (compared to different branches or tasks that manipulate the file

I understand where you're coming from, and generally, I would agree. However, I believe the case of dependency files might be an exception.

Regarding the dependency centralization issue, as it stands, we already have the Add Python Magic to requirements task, which modifies the requirements file on-the-fly to ensure a certain dependency. This means we are already looking at more than one place for understanding our dependencies.

The proposed solution might create similar challenges to those we face with the requirements.txt variable-defined. There would still be as many places to check as there are different sets of modules we install. The main difference is whether we look in the files folder or in the Ansible variable folders.

@enricostano
Copy link
Contributor Author

Hi @oyale ,

thanks for your inputs. As we already discussed the majority of the items you ask about in the previous session, I would wait for the next one to further clarify these points.

For the moment at Som Comunitats we'll use a fork of this project, waiting for this issue to get solved.

Thanks again,

Enrico

@oyale
Copy link
Contributor

oyale commented Aug 3, 2023

@enricostano

Would you mind sharing the minutes from the session you're referring to?

In any case, feel free to fork any of our code: it's Free Software. Forking instead answering the legitimate and reasoned concerns of peers is still valid.

However, I would prefer to maintain a spirit and attitude aligned with the values of cooperativism and open source, working on common projects that serve the needs of as many people as possible.

@enricostano
Copy link
Contributor Author

"I would wait for the next one to further clarify these points."

You probably missed this part, I hope to see you in the meeting!

@oyale
Copy link
Contributor

oyale commented Aug 3, 2023

I have read the whole message, thanks anyway, @enricostano.

I hope to see you in the meeting

Sure, just invite me and I'll do my best.

  • In advance, would you mind sending me the minutes of the previous meeting so that I have the necessary information to be able to make valuable contributions and save everyone's time at the meeting?

  • Answering the three key questions I've done above will also be helpfully.

@enricostano
Copy link
Contributor Author

@danypr92 @konykon would you mind to brief Lai about our session? Thanks!

@oyale
Copy link
Contributor

oyale commented Aug 3, 2023

They have already done so, however, they do not recall assessing what alternatives to pip tooling would be used or what the specific problems are that prevent the use of different requirements.txt files.

Let's please try to respect other people's time.

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.

4 participants