Skip to content
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

Refactor plugin loading #378

Merged
merged 22 commits into from
Feb 26, 2019
Merged

Refactor plugin loading #378

merged 22 commits into from
Feb 26, 2019

Conversation

kvosper
Copy link
Contributor

@kvosper kvosper commented Feb 22, 2019

The primary focus of this PR is to decouple the Admin server from the plugin-loading, so that will be able to visit the admin server before the plugins have all been loaded.
That work is not totally finished by this PR (as it is already quite large and we don't want to add even more changes).

I have also taken the opportunity to tidy-up parts of the plugin-loading code that were messy.

It is a prerequisite to allowing Styx to have proper liveness/readiness checks, which needs the admin server to be able to start without waiting for the plugins.

Copy link
Contributor

@dvlato dvlato left a comment

Choose a reason for hiding this comment

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

Additionally to my comments, I believe the description of the PR doesn't make it clear what the relevant changes are or why it is needed. Since it's quite a big PR I guess it would make sense to provide some guidance to people checking differences between versions and similar. (as well as for people reviewing the code!)

@kvosper kvosper merged commit 7fa76a1 into ExpediaGroup:master Feb 26, 2019
@kvosper kvosper deleted the refactor-plugin-loading branch February 26, 2019 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants