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

Added Configuration#validate_subscription! #190

Merged
merged 1 commit into from
Jul 21, 2016

Conversation

djberg96
Copy link
Collaborator

This adds a Configuration#validate_subscription! method, and addresses #189.

The main issue is that the constructor effectively does a validation check for tenant ID, client ID and client key via the fetch_providers method, but it does not validate the subscription. This method allows users to explicitly validate the subscription and/or check to ensure that it's enabled.

@djberg96
Copy link
Collaborator Author

@bzwei First attempt. What do you think?

@djberg96
Copy link
Collaborator Author

@bzwei @bronaghs Thoughts?

@bzwei
Copy link
Contributor

bzwei commented Jul 11, 2016

@djberg96 since configuration constructor already validate tenant ID, client ID/key, why don't we include validation of subscription ID there without making it an explicitly call for user?

@djberg96
Copy link
Collaborator Author

@bzwei I'm ok with an up-front validation, but it does mean an extra http request they wouldn't otherwise need to make.

@bzwei
Copy link
Contributor

bzwei commented Jul 11, 2016

I am voting for this solution because it makes the validation behaviors the same for all user provided IDs.

@djberg96 djberg96 force-pushed the validate_subscription branch 2 times, most recently from 9920f0b to b09ddae Compare July 13, 2016 18:43
@djberg96
Copy link
Collaborator Author

@bzwei I updated the code to validate the subscription in the constructor and updated the specs.

Automatically perform subscription_id validation in the constructor instead of making it a separate, optional method.

Update spec_helper.rb to stub validate_subscription, and update subscription_id test.
@miq-bot
Copy link
Member

miq-bot commented Jul 21, 2016

Checked commit djberg96@9af45f1 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 1 offense detected

lib/azure/armrest/configuration.rb

  • ❕ - Line 188, Col 7 - Metrics/AbcSize - Assignment Branch Condition size for validate_subscription is too high. [27.73/15]

@bzwei bzwei merged commit fc298a3 into ManageIQ:master Jul 21, 2016
@djberg96 djberg96 deleted the validate_subscription branch February 28, 2017 21:23
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.

None yet

3 participants