-
Notifications
You must be signed in to change notification settings - Fork 107
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
Make S3 a prototype feature #213
Conversation
@blomquisg hm, this will prevent it from being created. Problem is that we can't add it then from the UI, so that is probably an RFE for later. |
Looks like I have to add the Settings check to all of the tests, too... |
@blomquisg hm, you can probably just change the count of :ext_management_systems and only place where we need the setting enabled is https://github.com/Ladas/manageiq-providers-amazon/blob/05c2d067bbe36fba395211f5a9dd27bae7afdeec/spec/models/manageiq/providers/amazon/storage_manager/s3/stubbed_refresher_spec.rb |
5e6e20d
to
a6de22b
Compare
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.
Looks great. @durandom just asked me to do the 0 defaults, so that is super cool it's already done :-)
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.
Actually, could you run rubocop -a on those spec files? And push it as a separate commit please.
@@ -69,3 +69,6 @@ | |||
:ems_refresh_worker_amazon_network: {} | |||
:ems_refresh_worker_amazon_ebs_storage: {} | |||
:ems_refresh_worker_amazon_s3: {} | |||
:prototype: |
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.
note that settings will be merged with the core configs...
is there already a top level prototype key?
otherwise I'd put it underems -> ems_amazon -> s3_prototype
e.g.
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.
Yeah, there's already a prototype key in core settings.
Add a configuration to the Settings.yml file that allows administrators to enable or disable Amazon S3 support. The feature will be disabled by default. Disabling S3 will disable the S3 manager, which will prevent the S3 refresh worker from starting.
S3 tests will now only run is Settings.prototype.amazon.s3 is true.
a6de22b
to
87f80dc
Compare
Checked commits blomquisg/manageiq-providers-amazon@a419667~...0c2faca with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
@Ladas cops updated, and tests passing. |
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.
looks great now 👍
Make S3 a prototype feature (cherry picked from commit 2b92588)
Fine backport details:
|
Add a configuration to the Settings.yml file that allows administrators to enable or disable Amazon S3 support.
The feature will be disabled by default.
Disabling S3 will disable the S3 manager, which will prevent the S3 refresh worker from starting.