fixes #19659 - capsule sync via bulk action #6861
Conversation
|
Issues: #19659 |
bbuckingham
changed the title
bbuckingham
force-pushed
the
19659
branch
2 times, most recently
from
86648b0
to
bd53bf2
Compare
bbuckingham
changed the title
|
[test] |
|
[test] |
|
Getting this error after a sync but not sure if its related to your PR |
|
@johnpmitsch i've seen that before, try restarting your dev env |
| end | ||
|
|
||
| def humanized_input | ||
| ["'#{input['smart_proxy']['name']}'"] + super | ||
| end |
There was a problem hiding this comment.
Showing the name of the proxy in the task 👍 💯
| @@ -51,7 +51,7 @@ def remove_lifecycle_environment | |||
| param :environment_id, Integer, :desc => N_('Id of the environment to limit the synchronization on') | |||
| def sync | |||
| find_environment if params[:environment_id] | |||
| task = async_task(::Actions::Katello::CapsuleContent::Sync, capsule_content, :environment => @environment) | |||
| task = async_task(::Actions::Katello::CapsuleContent::Sync, capsule_content.capsule, :environment_id => @environment.try(:id)) | |||
There was a problem hiding this comment.
Why the change to environment_id? It seems like it's just used to find the environment below in app/lib/actions/katello/capsule_content/sync.rb (same question goes for the other ids too)
There was a problem hiding this comment.
With BulkAction, attempting to pass a hash containing AR resources resulted in plan phase errors similar to below; however, passing in a hash of IDs worked fine.
undefined method `to_hash' for #<Katello::ContentView:0x007f70248aa480>
I haven't found an alternative approach for passing in the resources, aside from passing in an array of arguments vs a hash. If you have suggestions, please do let me know. In most of Katello's usage of BulkAction, it does pass an array of arguments; however, I leaned away from it here, since we invoke the action differently depending upon scenario (e.g. cv + env vs repo).
| repository_id = options.fetch(:repository_id, nil) | ||
| repository = ::Katello::Repository.find(repository_id) unless repository_id.nil? | ||
| content_view_id = options.fetch(:content_view_id, nil) | ||
| content_view = ::Katello::ContentView.find(content_view_id) unless content_view_id.nil? |
There was a problem hiding this comment.
This is more readable and I think would work the same
content_view = ::Katello::ContentView.find(content_view_id) if content_view_id
| @@ -444,7 +444,8 @@ class CapsuleGenerateAndSyncTest < TestBase | |||
|
|
|||
| it 'plans' do | |||
| plan_action(action, repository) | |||
| assert_action_planed_with(action, ::Actions::Katello::CapsuleContent::Sync, capsule_content, :repository => repository) | |||
| assert_action_planned_with(action, ::Actions::BulkAction, ::Actions::Katello::CapsuleContent::Sync, | |||
| [capsule_content.capsule], :repository_id => repository.id) | |||
| end | |||
There was a problem hiding this comment.
How hard would it be to test two proxies syncing using bulk actions? That is some new functionality added by this PR and it would be nice to have the test coverage.
|
When I shut down http on one proxy, the other one kept syncing, 👍 |
Moving the capsule sync to a bulk action. This will ensure that if one capsule is down, that the remaining syncs continue.
|
@johnpmitsch, Thanks for the feedback. I pushed several updates based upon the comments. I haven't found a good alternative yet to passing IDs vs resources for the BulkAction. We can revisit if folks have other suggestions. |
|
Thanks @johnpmitsch! Created http://projects.theforeman.org/issues/20323 for the ui enhancement. |
Moving the capsule sync to a bulk action.
This will ensure that if one capsule is down, that
the remaining syncs continue.