Skip to content

[9.0][IMP] server_environment: Update Error Guards#672

Closed
tedsalmon wants to merge 1 commit into
OCA:9.0from
LasLabs:9.0-guard-server-environment-exceptions
Closed

[9.0][IMP] server_environment: Update Error Guards#672
tedsalmon wants to merge 1 commit into
OCA:9.0from
LasLabs:9.0-guard-server-environment-exceptions

Conversation

@tedsalmon

Copy link
Copy Markdown
  • Convert Exceptions to EnvironmentErrors
  • Move config checking into function _load_config()
  • Guard execution of _load_config() against EnvironmentErrors and log info
  • Update exception verbiage

* Convert Exceptions to EnvironmentErrors
* Move config checking into function `_load_config()`
* Guard execution of `_load_config()` against EnvironmentErrors and log info
* Update exception verbiage
@tedsalmon tedsalmon changed the title [IMP] server_environment: Update Error Guards [9.0][IMP] server_environment: Update Error Guards Dec 23, 2016

@lasley lasley left a comment

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.

Yes, exceptions out of global namespace. Thanks @t3ddftw 👍

The exceptions should be translated though IMO.

@lasley lasley added this to the 9.0 milestone Dec 24, 2016

@sbidoul sbidoul left a comment

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.

It is important that a missing running_env remains a hard error preventing the server to start, rather than a quiet log entry. At first glance this PR changes this behaviour.

@lasley

lasley commented Dec 26, 2016

Copy link
Copy Markdown
Contributor

@sbidoul - Without this change, simply cloning the server-tools repo into Odoo breaks installations badly. It seems to me that no matter the reasoning, breaking existing installations with modules that are not installed should not happen. Am I incorrect in this evaluation?

@sbidoul

sbidoul commented Dec 26, 2016

Copy link
Copy Markdown
Member

@lasley I thought this particular issue you mention was resolved by #670. This PR seems to go further. IMO, preventing the server to start when running_env is absent is a design feature of server_environment and must be preserved.

@lasley

lasley commented Dec 26, 2016

Copy link
Copy Markdown
Contributor

@sbidoul - That only partially resolves the issue because the Exception is still raised inside of the global namespace if the import passes. Odoo still attempts the module load, which ends us with an emergency line call in the middle of the night/early morning.

I see your #676 & will review that. Thanks!

@tedsalmon

Copy link
Copy Markdown
Author

@lasley,

The changes in #676 indeed resolve the issue. I'll go ahead and close this PR :).

Thanks!

@tedsalmon tedsalmon closed this Dec 27, 2016
@tedsalmon
tedsalmon deleted the 9.0-guard-server-environment-exceptions branch December 27, 2016 17:42
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (12.0)
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.

3 participants