Guideline to prohibit or allow the addition of dependencies on stable modules

[legalsylvain]

Hi all,

Sometimes, contributors proposes a PR that changes the dependency of a module. A recent case is here : OCA/server-ux#963, that propose to make server_action_mass_edit 16.0 depends on onchange_helper. For the time being, AFAIK, there is no OCA rules that forbid such changes. Some sentences in the guidelines might suggest that this has been authorised. Exemple:

When introducing breaking changes, it is expected that you include instructions or scripts to facilitate migration on current installations. Ref

However, if I understood correctly, some people says it is forbidden because it can break production instance when updating module.

Here is the place to debate this subject !

My point of view on that topic:

• it should be allowed when it is interesting. (if it adds new interesting feature, if it generates a more robust and complete module, if it fixes a bug, etc.) at the condition that it doesn't force to install many modules by dependency. So add a new dependency to a module that depends on base or web only is OK for me, because all instances are using it. It so should be forbidden if it forces to install main application that we don't want. Theoritical exemple: In sale_triple_discount (OCA/sale-workflow), add a dependency to purchase_triple_discount will force to install the main purchase module. In that case, a glue module is the solution.

• it should be allowed when it allows to mutualize substantial amounts of code. A very recent case occured in OCA/geospatial repo: web_view_leaflet_map provides a new view to see any model on a map. For that purpose, it was introducing the leaflet.js library. (See PR two years ago : [ADD] web_view_leaflet_map (use leaflet.js library to have map views) geospatial#310) Recently, a new OCA/web module has been proposed here that provides a new map widget, based also on the leaflet.js library. To avoid to have many modules introducing the same library (possibly different versions, conflict, etc...) I splitted web_view_leaflet_map into web_view_leaflet_map and web_leaflet_lib and make web_view_leaflet_map depends on the new web_leaflet_lib module. This addition of a new dependency has been accepted on a existing module here two weeks ago.

• For some UX feature, it is possible to make a lazy dependency. That's the case for exemple for the module web_tree_dynamic_colored_field. For exemple, the OCA/margin-analysis module named product_pricelist_margin (see installation documentation) does'nt force to install web_tree_dynamic_colored_field, but extra UX feature will be available if the module is installed. Similar situation occures with purchase_order_product_recommendation or sale_order_product_recommendation that have a better UX experience if web_widget_numeric_step is installed. However, for python code, a lazy dependency generate codes that are fully untestable. Having parts of code that are completely untested and that are triggered if a module is installed seems risky and lowers code coverage, reducing the quality of the module.

I don't really understand how it can break production deployement.

Let's leave aside deployments using pip (which I'm not very familiar with). In this case, I think there's no problem because pip update will install the new dependencies. (@sbidoul or other acsone members, can you confirm this point?).

Let's take the case of deployment using git pulls with several repos. Indeed, if we only have the OCA/server-ux repo with server_action_mass_edit installed and the module now depends on the onchange_helper module which is in the OCA/server-tools repo which is not pulled, the deployment will fail.

However, for the moment I've seen 2 types of deployment:

• The ‘industrial’ method used for example at Akrétion which uses a CI/CD system. the order to deploy therefore goes through a merge of a PR on a main branch, or through a git tag (I don't remember exactly). This triggers the execution of a deployment on a test instance. If it passes, it goes into production.
• the ‘home-made’ method I have at GRAP. (We don't have the skills to administer an ‘industrial’ system, and we only have 3 Odoo instances). In our case, I deploy on a test server, and if it passes, I deploy in production.

Yes, if a dependency is added the test will fail, and we'll have to correct it by adding the reference to the repo or the module, but the Odoo error message is explicit, and the solution takes 2 minutes.
I don't see how a production instance could break without anyone noticing.
After that, test instances exist because unexpected things can happen.

Could those who are against this type of change explain their fears?

CC : @grindtildeath, @pedrobaeza, @hbrunn, @rousseldenis, @ivantodorovich, @etobella, @ValentinVinagre, @dreispt, @mmequignon

Thanks a lot for your remarks !

[rvalyi]

additional considerations:

• some modules are purposely merged early while they state clearly they are alpha or beta. With such new modules, I think it's okay to do breaking changes because the audience has been properly warned.
• some module authors or maintainers can be legit in not having to assume tons of new features or new dependencies in the modules they accepted to maintain for their customers. More code and more features means more bugs or more support and you can be legit in not willing to assume them.
• I think people should really consider who is for or against a given PR. Some time ago we used to say the OCA was a "do-ocracy". Of course we can fool ourselves that we are all equal and blablabla. Well then we will have to cope with astroturfing with random new contributors trying to hijack roadmaps or the OCA image, as I once gave a few examples this is even happening already and it's already draining our energy away in some repos (not in this specific PR though). Lets not forget open source projects are usually run by the people doing them: eventually people like it, contribute and contributions get merged. Eventually people don't like it an fork it and the best fork ends up winning. I mean this is open source 101 and even if we try to have democratic ceremomies in the OCA we cannot really go too much against what I would call the thermodynamic rules of open source. So IMHO when the big "benevolent dictators" doing incredible amount of work insist on some decisions, I think we should have the humility to cope with their decisions, eventually do what we really need in our PRs, but generally speaking try to make it easy for these community heros. Again trying to please 10 people doing 100x less work wouldn't really help to move the project forward, even if they were right. To put things in perspective I really think without half of what @pedrobaeza is doing I really think the OCA would just be dead. This remind me 10 years ago: a guy called Fabien Pinckaers was overworking like crazy to keep OpenERP running. One day he got tired, turned to VC and then the license changed, the whole open source thing became way much harder. I know dozens of such projects, I would say it's even the sad norm in open source projects. My 2 cents.

[hbrunn]

In my book:

stable
no new python dependencies, only new Odoo dependencies with same or higher maturity level (authors/maintainers can override as they see fit)

mature
no new python dependencies, no new Odoo dependencies (authors/maintainers should not override)

[hbrunn]

I was meaning to support your position: No new dependencies on mature addons, don't we agree there?

[pedrobaeza]

Yes, first of all, I wanted to put the comment on the other thread (just done). Sorry for mixing. And thanks for your support :)

About your statement, indeed, that's something assumed, but we can clearly put it on the guidelines. Anyway, the system can be tricked, as some modules may not be yet consider mature (because also nobody has risen up the development status), and thus, people use it as a strict rule. That's why always PSC or core contributors may have the right for the final word.

[anajuaristi]

+1 @hbrunn mature no new python dependencies, no new Odoo dependencies (authors/maintainers should not override) I would avoid any mayor changes in stable and mature modules on stable Odoo versions. It's a nightmare when tracebacks starts happening in production instances due to an OCA module mayor change. It can cause a lot of headaches. I would allow changes on PRs on module migrations accross Odoo's versions since migrating a customer's instance means reviewing almost everything in test environments so it's easier to fight with it. My2cents

[hbrunn]

sure, migrations should be free game (for as far as it makes sense technically of course), if that's not implied, let's add an explicit sentence about that

[legalsylvain]

Hi all. Thanks for your contribution ! The objective of this issue is to define a general rule. (of course, exceptions happen).

1. I understand Holger, Ana and Pedro's proposal to base our approach on maturity. However, at the moment, this field is not filled in, and more importantly, it's wrong. Here are the figures for version 16.0.

Type	Quantity	Percent
Alpha	156	6,78 %
Beta	274	11,91 %
UNDEFINED (=Beta)	1628	70,75 %
Production/Stable	158	6,87 %
Mature	85	3,69 %
Total	2301	100,00 %

If we apply the current interpretation, this means that 91% of the modules are intended for testing and pre-production and 9% for production. this is simply not true.
If we consider that by default (if the maturity value is not defined) a module is ‘stable’, then we have 18% of modules intended for testing and pre-production and 82% intended for production. This seems to me to be more in line with reality.
That said, I remember putting this idea to @sbidoul a few OCA days ago, but there didn't seem to be a consensus.

My fear is that defined a rule based on the concept of maturity (which is itself not mature!), we're not solving much.

2. It's a nightmare when tracebacks starts happening in production instances

Thanks @anajuaristi. Could you elaborate that point ? As said in my first message, I don't understand how production instances can be broken. The deployment workflow has to be stopped at pre-production / test stage. Thank you for clarifying things for me (we don't all have the same way of working).

[etobella]

It is true that it fails in pre-production if you have defined several stages but:

• It requires someone to check why it is failing (otherwise, the update can be applied automatically)
• No updates can be applied in the instance until you fix the issue
• You need to find the module, the repo where it is stored and then add it to your configuration
  • If you use pip it is automatic
  • If you use gitaggregate you might need to add a new repo on the repo path
  • If you use minimal modules (gitaggregate, but only the necessary modules) you need to add this new module on the configuration
• You need to modify pre-production and production with the new configuration

I have suffered several headaches for change decisions on base modules defined as Beta but used in a lot of places that are modified and a lot of other modules start to fail. Even in modules where I am maintainer and original author. Then someone needs to apply patches to fix this kind of changes. For example: OCA/edi#540 (comment)

For that reason, it is recommended to avoid this kind of changes. I have done it in modules that have low repercussion just to avoid this kind of issues (For example: OCA/sign#14 )

I don't understand the real problem on doing this kind of patch if so many people from several companies and environments have asked for it.

[sbidoul]

Elsewhere, we have established that OCA follows a SemVer approach to versioning.

So breaking changes of any kind are allowed under certain conditions, such as:

• avoid them if possible (and more so if the module is stable/mature)
• consider carefully and justify the reason, weight the pros and cons
• when decided that it is valuable:
  • it must be done with a major version bump
  • and be accompanied with means to reduce the pain (migration scripts, good docs, etc)

In that framework, to me, adding or changing dependencies should be considered as any other breaking change. So not forbidden, but also not to be done lightly.

[legalsylvain]

@rvalyi.

I find the 3rd point of your answer rather off-topic.

• The title of this issue is "define a new rule, and if so, which one? "
• Your answer (if I understand correctly) is: "Above the rules defined in the guidelines, there is the fact that one or a few OCA "benevolent dictators" / "community heros" can take any decision, even when they're wrong".

If I didn't understood correctly, could you reword ? If I understood correctly, could you provide a PR in the current guidelines repo to explicit that point.
Note : It's not an ironic proposal. It's an important topic and I'm not sure that the members of the OCA all agree with your vision, including some historic and important contributors.

@etobella : Thanks for your clarification. A last point that is not clear for me.

It requires someone to check why it is failing (otherwise, the update can be applied automatically)

I don't get how update is "applied automatically" in production, if test / pre-production fails. Or did I misunderstood your sentence ?

[pedrobaeza]

I'm getting tired of expending a lot of time contributing, and a lot of time trying to justify some actions and not being heard about some changes petitions that don't cost too much, so I'm on the edge of abandoning both things and go on my way.

[sbidoul]

To be clear my #193 (comment) above is a general point of view on the matter of dependencies and breaking changes, and not an opinion on the discussion on the PR that apparently triggered this particular thread (which I don't have time to delve in now).

For what it's worth, I do sincerely appreciate the work you do for OCA, Pedro. Negotiating solutions to diverse requirements is sometimes complex but normal in open source, and especially exacerbated in OCA where often ownership of modules if diffuse or shared among many people and companies.

[rvalyi]

I'm getting tired of expending a lot of time contributing, and a lot of time trying to justify some actions and not being heard about some changes petitions that don't cost too much, so I'm on the edge of abandoning both things and go on my way.

@legalsylvain this message from @pedrobaeza in perspective with this

perfectly illustrates what I mean. One may not like it but this is my experience and IMHO we should avoid at all cost draining main contributors energy.