default merged_uri should return the parameter not the attribute #18497
Conversation
|
@miq-bot add_label hammer/yes |
|
@miq-bot add_label bug |
|
@miq_bot add_label core |
There was a problem hiding this comment.
Unsure if my review review has any valid "weight" to it, so if you disagree, I can reverse the "Request changes" status.
That said, at least would like a minor discussion about how we should avoid this from happening again in the future. Code changes seems reasonable otherwise though.
|
@miq-bot add_label core |
jerryk55
force-pushed
the
refix_merged_uri
branch
3 times, most recently
from
15eaa13 to
59e2000
Compare
A previous PR changed merged_uri to return the attribute on the object rather than the passed in parameter. This is incorrect and is changed herein. Only Swift handles this differently.
Make the ignored URI attribute and actual parameter more meaningful.
Add tests to validate that MiqSchedule.verify_file_depot does the right thing with regard to calling FileDepot.merged_uri.
Tried to rename the "uri" attribute inadvertantly in previous incarnation of this test and broke it. Fixing now.
jerryk55
force-pushed
the
refix_merged_uri
branch
from
59e2000 to
f1e0d16
Compare
|
@roliveri pushing fix to spec test back out. This is approved by Nick L - if Travis passes it is ready to be merged as far as I'm concerned. You can look if you like |
|
Checked commits jerryk55/manageiq@a4e1447~...f1e0d16 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
default merged_uri should return the parameter not the attribute (cherry picked from commit c9ae719) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1693728
|
Hammer backport details: |
A previous PR changed merged_uri to return the attribute on the object
rather than the passed in parameter. This is incorrect and is changed
herein. Only Swift handles this differently.
Thie method gets called by MiqSchedule.verify_file_depot() which in turn gets called by the UI controller in two different situations - for an AdHoc DB backup, and for a Scheduled DB Backup. We had apparently only tested AdHoc backups previously, which is why this worked. However Scheduled backups were failing because the FileDepot object had not been created in the DB and therefore the URI attribute was nil, which is invalid when the object is subsequently created.
If we return the passed in uri parameter instead of the attribute, both Adhoc and Scheduled DB backups will work correctly.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1678223
@roliveri @h-kataria @NickLaMuro please review. Thanks.
Links