Skip to content

Don't expand plugins that are already unpacked #1226

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 26, 2017
Merged

Don't expand plugins that are already unpacked #1226

merged 2 commits into from
May 26, 2017

Conversation

binarin
Copy link
Contributor

@binarin binarin commented May 19, 2017

This prevents creating unnecessary files when running in embedded
mode, and in development mode it keeps code path clean and compatible
with different interactive tools.

binarin pushed a commit to binarin/rabbitmq-server that referenced this pull request May 19, 2017
Use plugins dist dir as primary source of truth about presen plugins
6f5be43 
This is a 'master' counterpart for rabbitmq#1226
@binarin binarin mentioned this pull request May 19, 2017
Merged
@dumbbell dumbbell self-assigned this May 22, 2017
dumbbell
dumbbell requested changes May 22, 2017
View reviewed changes
Copy link
Collaborator

@dumbbell dumbbell left a comment

Choose a reason for hiding this comment

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

I like the patch and it works fine here too.

However, the behavior is changed (in a good way IMHO): now it reports when it fails to extract the zip archive or when there is no .app file. Before, it would ignore those errors and skip the faulty plugins. Could you please note that in your commit message?

@hairyhum
Copy link
Contributor

Looks good. Though I'm a bit concerned about using plugins_dist_dir in the active/0 function, because it's unzipping an .app file every time. On other hand, the function is not widely used (only in plugin operation), so it should be fine.

Don't expand plugins that are already unpacked
ab66157 
This prevents creating unnecessary files when running in embedded
mode, and in development mode it keeps code path clean and compatible
with different interactive tools.

Also some new error conditions are properly handled and logged:
- Failure to unpack an .ez-file
- Missing .app file in plugin directory
@binarin binarin force-pushed the dont-expand-dir-plugins-stable branch from 9e78c09 to ab66157 Compare May 22, 2017 15:35
@binarin
Copy link
Contributor Author

binarin commented May 22, 2017

I've updated the commit message.

binarin pushed a commit to binarin/rabbitmq-server that referenced this pull request May 22, 2017
Use plugins dist dir as primary source of truth about presen plugins
2811aab 
This is a 'master' counterpart for rabbitmq#1226
@michaelklishin michaelklishin added this to the 3.6.11 milestone May 22, 2017
@michaelklishin
Copy link
Collaborator

We will postpone this until after 3.6.10 GA ships.

dumbbell
dumbbell approved these changes May 23, 2017
View reviewed changes
Copy link
Collaborator

@dumbbell dumbbell left a comment

Choose a reason for hiding this comment

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

Thank you for the updated commit message!

@michaelklishin
Copy link
Collaborator

michaelklishin commented May 23, 2017 •
edited
Loading

@binarin I updated a few error messages and tuple tags in fb17eed (#1227). Can you please update this PR accordingly? Thank you.

michaelklishin
michaelklishin requested changes May 23, 2017
View reviewed changes
Copy link
Collaborator

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

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

Needs a wording and naming update to be in sync with master (fb17eed)

@binarin
Copy link
Contributor Author

binarin commented May 23, 2017

I've cherry-picked fb17eed here.

@michaelklishin michaelklishin merged commit 9084194 into rabbitmq:stable May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelklishin michaelklishin michaelklishin approved these changes

@dumbbell dumbbell dumbbell approved these changes

Assignees

@dumbbell dumbbell

Labels
None yet
Projects
None yet
Milestone
3.6.11
Development

Successfully merging this pull request may close these issues.
4 participants
@binarin @hairyhum @michaelklishin @dumbbell