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.

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

Support firmware update #597

Merged
merged 1 commit into from
Jul 16, 2019
Merged

Support firmware update #597

merged 1 commit into from
Jul 16, 2019

Conversation

miha-plesko
Copy link
Contributor

With this commit we expose neccessary operations for UI to be able to render a functional "Firmware Update" modal. User has to pick a firmware binary from the drop-down to apply it to selected server(s).
The API call to support it looks like:

GET /api/physical_servers/{ID}/firmware_binaries

Eventually, when she submits the modal, a new Request of type PhysicalServerFirmwareUpdateRequest is created by means of POSTing to /api/request:

POST /api/requests
{
  "options" : {
    "request_type": "physical_server_firmware_update",
    "src_ids": [1,2,3],
    "firmware_binary_id": 1
  }
}

@miq-bot miq-bot added the wip label May 27, 2019
@miha-plesko miha-plesko mentioned this pull request May 31, 2019
Merged
8 tasks
@miha-plesko miha-plesko force-pushed the firmware-update branch 2 times, most recently from 6bb08e9 to 41a99e5 Compare May 31, 2019 13:36
@miha-plesko miha-plesko closed this Jul 8, 2019
@miha-plesko miha-plesko reopened this Jul 8, 2019
@miha-plesko miha-plesko changed the title [WIP] Support firmware update Support firmware update Jul 9, 2019
@miha-plesko
Copy link
Contributor Author

@miq-bot add_label enhancement
@miq-bot assign @abellotti

Hi @abellotti the dependent core PR has been merged so we'll need this API enhancement to implement the React UI. Kindly ask for a review.

@miq-bot miq-bot removed the wip label Jul 9, 2019
@lpichler
Copy link
Contributor

lpichler commented Jul 9, 2019

@miq-bot assign @lpichler

@miha-plesko is CI failure related to your PR ?

@miq-bot miq-bot assigned lpichler and unassigned abellotti Jul 9, 2019
@miha-plesko
Copy link
Contributor Author

Oh my, there's another core PR that needs to be merged first ManageIQ/manageiq#18801. Marking WIP until then again

@miha-plesko miha-plesko changed the title Support firmware update [WIP] Support firmware update Jul 9, 2019
@miq-bot miq-bot added the wip label Jul 9, 2019
@miha-plesko miha-plesko reopened this Jul 12, 2019
@miha-plesko miha-plesko changed the title [WIP] Support firmware update Support firmware update Jul 12, 2019
@miha-plesko
Copy link
Contributor Author

Hey @lpichler , un-wiping again because also the other dependent core PR has been merged yesterday.

@miq-bot miq-bot removed the wip label Jul 12, 2019
@miha-plesko
Copy link
Contributor Author

/cc @gtanzillo we need this before code freeze, kindly ask for your help.

lpichler
lpichler reviewed Jul 16, 2019
View reviewed changes
app/controllers/api/physical_servers_controller.rb Outdated
@@ -65,6 +65,10 @@ def apply_config_pattern_resource(type, id, data)
action_result(false, err.to_s)
end

def firmware_binaries_query_resource(object)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @miha-plesko

we used to put sub collections related methods to the Api::Subcollections::NameOfCollection module.
In your case probably something like Api::Subcollections::FirmwareBinaries. See Subcollections::EventStreams in this controller class as example.

Does make sense to put it here for you ?
thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, yes it makes sense. My "change as little as possible" was a bit too extream. Fixed

@lpichler
Copy link
Contributor

I have one comment, otherwise looks good to me 👍

@miha-plesko
Support firmware update
7abf6a5 
With this commit we expose neccessary operations for UI to be
able to render a functional "Firmware Update" modal. User has
to pick a firmware binary from the drop-down to apply it to
selected server(s). Eventually, when she submits the modal, a
new Request of type PhysicalServerFirmwareUpdateRequest is created
by means of POSTing to `/api/request`.

Signed-off-by: Miha Pleško <[email protected]>
@miq-bot
Copy link
Member

miq-bot commented Jul 16, 2019

Checked commit xlab-si@7abf6a5 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

gtanzillo
gtanzillo approved these changes Jul 16, 2019
View reviewed changes
Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gtanzillo gtanzillo added this to the Sprint 116 Ending Jul 22, 2019 milestone Jul 16, 2019
@gtanzillo gtanzillo merged commit 90de9e9 into ManageIQ:master Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpichler lpichler lpichler left review comments

@gtanzillo gtanzillo gtanzillo approved these changes

Assignees

@lpichler lpichler

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@miha-plesko @lpichler @miq-bot @gtanzillo @abellotti