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

v2v Throttling #18415

Merged
merged 18 commits into from
Feb 27, 2019
Merged

v2v Throttling #18415

merged 18 commits into from
Feb 27, 2019

Conversation

jameswnl
Copy link
Contributor

@jameswnl jameswnl commented Jan 30, 2019

Moving v2v throttling our of automate

These slides may help understand the changes

Related to ManageIQ/manageiq-content#504 (Corresponding Automate code being replaced/updated)

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1686155

@jameswnl jameswnl force-pushed the throttle branch 2 times, most recently from 2cda9bc to 92eb47c Compare January 30, 2019 21:18
@miq-bot miq-bot added the wip label Jan 30, 2019
@miq-bot
Copy link
Member

miq-bot commented Jan 30, 2019

This pull request is not mergeable. Please rebase and repush.

end

def migration_task
@migration_task ||= target_entity
Copy link
Member

Choose a reason for hiding this comment

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

The target_entity is known at initialization time right? This looks like it could just be an attr_reader that is set in initialize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is caching the query result of target_entity. Shall I rely on rails to take care of caching, and just do a alias_method :migration_task, :target_entity?

Copy link
Member

Choose a reason for hiding this comment

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

Oh no problem with the caching, I was saying in initialize you could do @migration_task = target_entity and make migration_task an attr_reader which is basically what this duplicates the behavior of.

@miq-bot
Copy link
Member

miq-bot commented Feb 22, 2019

Checked commits jameswnl/manageiq@4709864~...0b1a8c5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
9 files checked, 7 offenses detected

app/models/infra_conversion_job.rb

@agrare agrare merged commit e3e7c38 into ManageIQ:master Feb 27, 2019
@agrare agrare added this to the Sprint 106 Ending Mar 4, 2019 milestone Feb 27, 2019
@jameswnl
Copy link
Contributor Author

jameswnl commented Mar 6, 2019

@miq-bot add_lable hammer/yes

@miq-bot
Copy link
Member

miq-bot commented Mar 6, 2019

@jameswnl unrecognized command 'add_lable', ignoring...

Accepted commands are: add_label, add_reviewer, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

@jameswnl
Copy link
Contributor Author

jameswnl commented Mar 6, 2019

@miq-bot add_label hammer/yes

@jameswnl jameswnl mentioned this pull request Mar 6, 2019
simaishi pushed a commit that referenced this pull request Mar 8, 2019
@simaishi
Copy link
Contributor

simaishi commented Mar 8, 2019

Hammer backport details:

$ git log -1
commit d1585b3a59c31d58ee0412fa8ef4daa5c1a26df2
Author: Adam Grare <agrare@redhat.com>
Date:   Wed Feb 27 15:46:02 2019 -0500

    Merge pull request #18415 from jameswnl/throttle
    
    v2v Throttling
    
    (cherry picked from commit e3e7c38b24a445f50ded7bf15438fadd0da9294a)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1686877

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