Skip to content

[16.0][ADD] fs_storage_environment: make server_environment an optional dependency#574

Draft
yankinmax wants to merge 2 commits intoOCA:16.0from
camptocamp:16-dr-fs_storage
Draft

[16.0][ADD] fs_storage_environment: make server_environment an optional dependency#574
yankinmax wants to merge 2 commits intoOCA:16.0from
camptocamp:16-dr-fs_storage

Conversation

@yankinmax
Copy link
Copy Markdown

PR is aimed to remove server_environment dependency from fs_storage in the scope of this issue:

Copy link
Copy Markdown
Contributor

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

Thanks @yankinmax !

"data": [
"views/fs_storage_view.xml",
],
"auto_install": True,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need a migration script in fs_storage to get fs_storage_environment automatically installed on existing deployments? or is the auto_install: True enough?

@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Mar 25, 2026

@yankinmax It's a breaking change on a critical addon. It would have been nice to have a discussion about this before making such a change. If someone using environment conf updates the fs_storage addon to the new version without having the fs_storage_environment into its addon path, it will break its installation without any error message explaining why. I'm a little bit afraid by such a change.
server_environment is also a depency for

  • fs_attachment
  • fs_attachment_s3
  • fs_storage_backup

Copy link
Copy Markdown
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

If you remove the dependency on server_environment at the fs_storage level you also need to create glue addons for:

  • fs_attachment
  • fs_attachment_s3
  • fs_storage_backup

But are we ready for such a change? I could manage it on my own projects but these addons are widely used. I think it’s important to gather as much feedback as possible before deciding whether to make this change across all versions or focus on the most recent one.

@yankinmax
Copy link
Copy Markdown
Author

Hello @lmignon I completely understand the situation.
I've opened this PR, but still need to create a migration script to force the new module installation.

"endpoint_url": "$AWS_ENDPOINT_URL",
}
""",
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This part eval_options_from_env can stay, it is not related to server_environment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indeed

@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Mar 25, 2026

Hello @lmignon I completely understand the situation. I've opened this PR, but still need to create a migration script to force the new module installation.

It’s important to keep in mind the different installation methods used by the different integrators. There’s no guarantee that the glue modules will be in the addon's path even if the new version of fs_storage is installed (this is currently the case when dependencies are managed by pip for exemple).

@ivantodorovich
Copy link
Copy Markdown
Contributor

Hello @lmignon I completely understand the situation. I've opened this PR, but still need to create a migration script to force the new module installation.

It’s important to keep in mind the different installation methods used by the different integrators. There’s no guarantee that the glue modules will be in the addon's path even if the new version of fs_storage is installed (this is currently the case when dependencies are managed by pip for exemple).

How do you recommend we proceed?
We do need to remove this hard dependency

Usually auto-installing the newly created glue module and ensuring the transition is smooth is enough -- then, custom deployment details are a responsibility of each integrator. The same thing occurs when new python dependencies are added to existing modules if you're not using pip to install odoo addons.

@yankinmax
Copy link
Copy Markdown
Author

Hello reviewers, after trying several approaches to write migration script we've decided to investigate further the server_environment removal from fs_storage.

I'll put this and related PR's to draft for now.
Thank you for taking time to discuss and review it.

@yankinmax yankinmax marked this pull request as draft March 25, 2026 12:55
@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Mar 25, 2026

Hello @lmignon I completely understand the situation. I've opened this PR, but still need to create a migration script to force the new module installation.

It’s important to keep in mind the different installation methods used by the different integrators. There’s no guarantee that the glue modules will be in the addon's path even if the new version of fs_storage is installed (this is currently the case when dependencies are managed by pip for exemple).

How do you recommend we proceed? We do need to remove this hard dependency

Usually auto-installing the newly created glue module and ensuring the transition is smooth is enough -- then, custom deployment details are a responsibility of each integrator. The same thing occurs when new python dependencies are added to existing modules if you're not using pip to install odoo addons.

@yankinmax @ivantodorovich
On stable branches we try to avoid to introduce new dependencies but I'm not a fan of enforcing such a rule. Your refactoring could be a good idea and a new approach of server_environement also. To lower the risk and also automate the auto discovery/installation of the new glue addons declared as auto_install for people using pip, @sbidoul suggested and improvement into the lib used to package addons as standard python wheel. (sbidoul/whool#40). I think we'll find a path to lower the risk of such a change...

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