Config: add the hidden warnings.rabbitmq_version option
#5415
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sphuber
force-pushed
the
fix/5414/config-option-disable-rabbitmq-warning
branch
2 times, most recently
from
7f0387f to
a7ef8c2
Compare
chrisjsewell
suggested changes
Mar 8, 2022
aiida/manage/manager.py
Outdated
| @@ -445,12 +445,18 @@ def check_rabbitmq_version(communicator: 'RmqThreadCommunicator'): | |||
| from packaging.version import parse | |||
|
|
|||
| from aiida.cmdline.utils import echo | |||
| from aiida.manage.configuration import get_config_option | |||
|
|
|||
| show_warning = get_config_option('warnings.rabbitmq_version') | |||
There was a problem hiding this comment.
Suggested change
| show_warning = get_config_option('warnings.rabbitmq_version') | |
| show_warning = self.get_option('warnings.rabbitmq_version') |
There was a problem hiding this comment.
(FYI this is the code of get_config_option: return get_manager().get_option(option_name), i.e. it is just a shortcut)
|
Eurgh, this feels like unnecessary extra complexity, just for the sake of hiding one config option, without much testing. IMHO, I would trust users to know what they are doing, if they set this to false. |
|
I implemented this as a quick proof-of-concept because when I suggested adding this option @giovannipizzi requested it be hidden. I would personally also be fine with just having this as a normal option and leave it up to the user's discretion whether to ignore it. |
sphuber
force-pushed
the
fix/5414/config-option-disable-rabbitmq-warning
branch
from
a7ef8c2 to
a6080e5
Compare
sphuber
force-pushed
the
fix/5414/config-option-disable-rabbitmq-warning
branch
from
a6080e5 to
08a4f3f
Compare
|
@chrisjsewell removed the first commit that added concept of hidden options. Also addressed your other comment and fixed the conditional of the correct RabbitMQ version that we missed in the other PR. |
chrisjsewell
approved these changes
Mar 9, 2022
|
Well, its a pre-approval, for when the tests are fixed |
sphuber
force-pushed
the
fix/5414/config-option-disable-rabbitmq-warning
branch
from
08a4f3f to
4d5ef06
Compare
This option can be used to suppress the warning that is emitted when a
profile storage is loaded that is configured with a version of RabbitMQ
that is known to be able to cause problems. The option is hidden because
the warning is very important and should not easily be ignorable by
normal users, but it is mostly intended for developers.
The `check_rabbitmq_version` and `check_version` free functions are
moved to `Manager` as methods which allows to use `get_option` instead
of the `get_config_option` free function which is just an indirect way
of calling `Manager.get_option`.
The `is_rabbitmq_version_supported` function is also corrected. It was
still using `< parse('3.8')` as a condition whereas it should be
`< parse('3.8.15')` which was recently already changed for the
`check_rabbitmq_version` function.
sphuber
force-pushed
the
fix/5414/config-option-disable-rabbitmq-warning
branch
from
4d5ef06 to
f4fda1f
Compare
sphuber
added a commit
to sphuber/aiida-core
that referenced
this pull request
Mar 24, 2022
This option can be used to suppress the warning that is emitted when a profile storage is loaded that is configured with a version of RabbitMQ that is known to be able to cause problems. The `check_rabbitmq_version` and `check_version` free functions are moved to `Manager`. The `is_rabbitmq_version_supported` function is removed as it is no longer used. Cherry-pick: 9dd216f
sphuber
added a commit
that referenced
this pull request
Mar 24, 2022
This option can be used to suppress the warning that is emitted when a profile storage is loaded that is configured with a version of RabbitMQ that is known to be able to cause problems. The `check_rabbitmq_version` and `check_version` free functions are moved to `Manager`. The `is_rabbitmq_version_supported` function is removed as it is no longer used. Cherry-pick: 9dd216f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5414
This option can be used to suppress the warning that is emitted when a
profile storage is loaded that is configured with a version of RabbitMQ
that is known to be able to cause problems. The option is hidden because
the warning is very important and should not easily be ignorable by
normal users, but it is mostly intended for developers.
The
check_rabbitmq_versionandcheck_versionfree functions aremoved to
Manageras methods which allows to useget_optioninsteadof the
get_config_optionfree function which is just an indirect wayof calling
Manager.get_option.The
is_rabbitmq_version_supportedfunction is also corrected. It wasstill using
< parse('3.8')as a condition whereas it should be< parse('3.8.15')which was recently already changed for thecheck_rabbitmq_versionfunction.