Skip to content

[18.0][ADD] webservice_server_env: make server_environment an optional dependency - #146

Open
yankinmax wants to merge 3 commits into
OCA:18.0from
camptocamp:remove-server-env-dep-webservice
Open

[18.0][ADD] webservice_server_env: make server_environment an optional dependency#146
yankinmax wants to merge 3 commits into
OCA:18.0from
camptocamp:remove-server-env-dep-webservice

Conversation

@yankinmax

@yankinmax yankinmax commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @etobella,
some modules you are maintaining are being modified, check this out!

@OCA-git-bot OCA-git-bot added mod:webservice Module webservice series:18.0 mod:webservice_environment Module webservice_environment labels Jul 9, 2026
@yankinmax yankinmax changed the title Remove server env dep webservice [18.0][ADD] webservice_environment: make server_environment an optional dependency Jul 9, 2026
@yankinmax

Copy link
Copy Markdown
Contributor Author

Hello @dreispt @ivantodorovich @simahawk can you pls take a look?

@yankinmax
yankinmax force-pushed the remove-server-env-dep-webservice branch from 0ee7047 to 4a4462c Compare July 9, 2026 15:13
@OCA-git-bot OCA-git-bot added mod:webservice_server_env Module webservice_server_env and removed mod:webservice_environment Module webservice_environment labels Jul 9, 2026
@yankinmax yankinmax changed the title [18.0][ADD] webservice_environment: make server_environment an optional dependency [18.0][ADD] webservice_server_env: make server_environment an optional dependency Jul 9, 2026
@yankinmax
yankinmax force-pushed the remove-server-env-dep-webservice branch from 4a4462c to cf1e2e4 Compare July 10, 2026 05:46
Comment thread test-requirements.txt Outdated
@yankinmax
yankinmax force-pushed the remove-server-env-dep-webservice branch from cf1e2e4 to 59f2570 Compare August 10, 2026 12:29
@ivantodorovich

Copy link
Copy Markdown

@simahawk @dreispt

@yankinmax

Copy link
Copy Markdown
Contributor Author

@ivantodorovich I've updated this PR with backport commit of uninstall hook to be consistent with v19

@yankinmax

Copy link
Copy Markdown
Contributor Author

Hello @simahawk @etobella @dreispt , can you pls take a look and possibly trigger merge?

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[("name", "=", "webservice_server_env"), ("state", "=", "uninstalled")]
)
if module:
module.button_install()

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.

in the case that the module is not added, can you provide the uninstall hook to pass all data?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The uninstall hook is here: b175404

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.

in the migration should be there if the other module is not installed (it does not exist)

in some cases that could happen if you only add the modules

Copy link
Copy Markdown
Contributor 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?
In general we should have two cases:

  1. webservice is already installed in the database (meaning current database version of the module depends on server_environment): then webservice upgrade will remove the dependency and upgrade hook will install webservice_server_env.
  2. webservice is not installed and this is a brand new installation: if server_environment is installed then webservice_server_env will be auto installed, because of auto_install key in manifest.
    In both cases if you decide to uninstall server_environment and webservice_server_env the uninstall hook will make the fields regular for the webservice.

Can you pls describe the case I need to handle?

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.

No, there is a 3rd case (and it happened to me with fs_storage).

You have server_environment and webservice installed. On the update, you don't know that you need the new module, and you don't add it (I use doodba, and I specify exactly the modules I need). On the update, the if is false, as the module is not found, then, you loose all data, leaving your instance without the credentials.

We have 2 options:

1- raise an exception if the module is not found
2- Make it in a way that the module restores data in case the module is not found.

@yankinmax yankinmax Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You have server_environment and webservice installed. On the update, you don't know that you need the new module, and you don't add it

I thought it's actually covered by Odoo with auto_install: if you potentially specify the list of modules to install, the web-api submodule is present on a branch revision (or even aggregated one is built on top of the branch revision), so you definitely have in the code source the webservice_server_env, then even if you don't force it in the list it should be auto installed.
Am I not right?

@ivantodorovich @simahawk @gurneyalex WDYT?

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.

Yes, but that implies that the module is there. A lot of people is adding the whole branch of modules of the OCA repository, but that is not mandatory. That happened to me, with a system that uses git-aggregate and a process to get only the necessary modules. That could happen to if you use PyPi too. Doing this kind of activities (limiting the modules) allows us to be sure that the modules in the instance are compatible and allows us to make some tests on the instance before pushing it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, I see....but I still can't get what kind of uninstall hook I need to add. Can you pls suggest the pseudo-code idea?

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.

Something like this:

https://github.com/OCA/web-api/pull/146/changes#diff-477d83525842ed4ad9af6ca90b3a4d7c434ed43d33a48065d35662c006445572

Another option is to raise an Exception, this way this problem is avoided.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@etobella WDYT:

def migrate(env, version):
    module = env["ir.module.module"].search([("name", "=", "webservice_server_env")])
    if not module:
        raise exceptions.UserError(
            _(
                "The 'webservice_server_env' module is not available. "
                "It is required to preserve the server environment managed "
                "fields of 'webservice.backend'. Make it available on the "
                "addons path before upgrading 'webservice'."
            )
        )
    if module.state == "uninstalled":
        module.button_install()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants