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

Extend the existing provider creation with DDF support #723

Merged
merged 1 commit into from
Feb 7, 2020

Conversation

skateman
Copy link
Member

@agrare created a new method on the amazon provider that creates a new record from the data submitted from a DDF. In order to keep backwards compatibility and also have the fancy POST /api/providers for creation, I added a test for a ddf attribute. If this attribute is set, the new creation method will be called and if not, the legacy one is executed. So it is necessary to pass a ddf: true attribute with the submitted values.

Parent issue: ManageIQ/manageiq#18818

@miq-bot assign @martinpovolny
cc @Hyperkid123 can you test it together with ManageIQ/manageiq-providers-amazon#580 in your code?

@Hyperkid123
Copy link
Contributor

@skateman first thing on Monday

@Hyperkid123
Copy link
Contributor

Hyperkid123 commented Jan 13, 2020

@skateman I am having an issue with submitting a zone:

error: {kind: "bad_request",…}
kind: "bad_request"
message: "Could not create the new provider - Zone(#47275658611780) expected, got "4" which is an instance of String(#47275612667420)"
klass: "Api::BadRequestError"

I am also getting this error in the amazon provider plugin when doing the options query (not on master only with Adams PR):

LoadError (Unable to autoload constant ManageIQ::Providers::Amazon::StorageManager::Ebs, expected /home/mmarosi/manageiq/manageiq-providers-amazon/app/models/manageiq/providers/amazon/storage_manager/ebs.rb to define it):

Maybe I am missing some other dependency?

@skateman
Copy link
Member Author

No, I guess the zone field needs a dataType set to an integer and maybe its name should be zone_id instead of zone.

@skateman
Copy link
Member Author

@skateman
Copy link
Member Author

Also make sure you're loading Adam's PR mentioned above

@Hyperkid123
Copy link
Contributor

Umm, no ... you need the name of the zone, not the ID. See here https://github.com/ManageIQ/manageiq-providers-amazon/pull/580/files#diff-235184baadf210fc9a4d037d8ffa1c37R179

What? Using the name instead of ID? I guess I can try it.

Also make sure you're loading Adam's PR mentioned above

I am using all the PRs mentioned

@skateman
Copy link
Member Author

Okay, try it. The autoloading error is real, I sent it over to Adam. But he's on PTO this week.

@skateman
Copy link
Member Author

@Hyperkid123 I made some changes, if you send the zone as a string, it goes further. I guess we need the list of the regions in that dropdown to send a correct one, because that's where it's failing now...

@Hyperkid123
Copy link
Contributor

@skateman the regions currently are using the same values as in the original code. We can change that easily.

@skateman
Copy link
Member Author

skateman commented Feb 7, 2020

After some hacking around in the UI, I was able to create a form that uses this as an endpoint and successfully created a provider, so this is good to go @lpichler

I even added tests to make you happy 😉

@skateman skateman changed the title [WIP] Extend the existing provider creation with DDF support Extend the existing provider creation with DDF support Feb 7, 2020
@skateman
Copy link
Member Author

skateman commented Feb 7, 2020

@miq-bot add_label enhancement

@miq-bot
Copy link
Member

miq-bot commented Feb 7, 2020

Checked commit skateman@4139a83 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@lpichler lpichler added this to the Sprint 130 Ending Feb 17, 2020 milestone Feb 7, 2020
@lpichler lpichler merged commit 644f4af into ManageIQ:master Feb 7, 2020
@skateman skateman deleted the provider-ddf-create branch February 7, 2020 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants