-
Notifications
You must be signed in to change notification settings - Fork 4k
Change the default location of the 'enabled_plugins' file #2298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bb604c8 to
9927b60
Compare
|
This PR should fix this issue - |
dumbbell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't test the patch locally yet, but beside my comment, it looks good to me.
However I'm still not sure about the approach, but I will add a comment in the #2234 issue.
| rabbit_env:log_context(Context2), | ||
|
|
||
| %% Migrate the enabled_plugins file to the new location if necessary | ||
| Context3 = rabbit_plugins:maybe_migrate_enabled_plugins_file(Context2), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rabbitmq_prelaunch application can't depend on a module from rabbit. The dependency is the other way around.
You could move the code you added to rabbit_plugins to a new rabbitmq_prelaunch_plugins module in the rabbitmq_prelaunch application for instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that makes sense.
I see we don't appear to separate the prelaunch app's tests into their own directory? So I will leave the rabbit_plugins_SUITE in test/ as rabbit_prelaunch_plugins_SUITE instead of placing it in apps/rabbit_prelaunch/test.
During prelaunch, if an explicit plugins file location has not been specified via the RABBITMQ_ENABLED_PLUGINS_FILE env var, adopt a new location of /var/lib/rabbitmq/enabled_plugins if possible, but fallback to the original location if there is no file at the new location, and the file at the old location is both readable and writable. No change is applied for Windows.
apps/rabbit_prelaunch should not depend on rabbit, since rabbit already depends on apps/rabbit_prelaunch. So, rabbit_plugins:maybe_migrate_enabled_plugins_file/1 becomes rabbit_prelaunch_plugins:maybe_migrate_enabled_plugins_file/1.
3852035 to
fc18ca9
Compare
|
My recollection was that there were some additional unwanted side effects stemming from these changes. Since has been sitting for quite some time, I think the issue is best closed. |
During prelaunch, if an explicit plugins file location has not been specified via the
RABBITMQ_ENABLED_PLUGINS_FILEenv var, adopt a new location of/var/lib/rabbitmq/enabled_pluginsif possible, but fallback to the original location if there is no file at the new location, and the file at the old location is both readable and writable. It the file at the old location is strictly readable, we copy it to the new location and log a message.No change is applied for Windows, since the Windows
config_base_diranddata_dirdefault to the same location and there is effectively no change.