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

Simplified VmScan base class as all the provider logic has moved into relevant subclasses #19607

Merged
merged 4 commits into from
Jan 6, 2020

Conversation

chessbyte
Copy link
Member

@chessbyte chessbyte commented Dec 6, 2019

Related PRs:

Cross-repo tests here

Leveraging this PR, the new state machine for the base class looks like this.

vm_scan

@chessbyte chessbyte changed the title Simplified base class as all the provider logic has moved into relevant subclasses Simplified VmScan base class as all the provider logic has moved into relevant subclasses Dec 6, 2019
@chessbyte
Copy link
Member Author

@hsong-rh when you get a chance, can you verify that this PR works with Amazon SmartState?

app/models/vm_scan.rb Outdated Show resolved Hide resolved
@chessbyte chessbyte changed the title Simplified VmScan base class as all the provider logic has moved into relevant subclasses [WIP] Simplified VmScan base class as all the provider logic has moved into relevant subclasses Dec 11, 2019
@miq-bot miq-bot added the wip label Dec 11, 2019
@chessbyte chessbyte force-pushed the subclass_vm_scan branch 4 times, most recently from cdc5d34 to 3a06cd9 Compare December 12, 2019 21:02
@agrare
Copy link
Member

agrare commented Jan 6, 2020

@chessbyte I'm getting the following error testing VMware SmartState:

[----] E, [2020-01-06T12:23:48.913698 #18213:2af460e245bc] ERROR -- : Q-task_id([job_dispatcher]) [NoMethodError]: undefined method `join' for nil:NilClass  Method:[block (2 levels) in <class:LogProxy>]
[----] E, [2020-01-06T12:23:48.913793 #18213:2af460e245bc] ERROR -- : Q-task_id([job_dispatcher]) /home/agrare/src/manageiq/manageiq/app/models/mixins/scanning_mixin.rb:122:in `sync_metadata'
/home/agrare/src/manageiq/manageiq/app/models/vm_scan.rb:137:in `call_synchronize'
/home/agrare/src/manageiq/manageiq-providers-vmware/app/models/manageiq/providers/vmware/infra_manager/scanning/job.rb:257:in `snapshot_complete'
/home/agrare/src/manageiq/manageiq/app/models/job/state_machine.rb:48:in `signal'
/home/agrare/src/manageiq/manageiq-providers-vmware/app/models/manageiq/providers/vmware/infra_manager/scanning/job.rb:56:in `call_snapshot_create'
/home/agrare/src/manageiq/manageiq/app/models/job/state_machine.rb:48:in `signal'
/home/agrare/src/manageiq/manageiq-providers-vmware/app/models/manageiq/providers/vmware/infra_manager/scanning/job.rb:23:in `before_scan'
/home/agrare/src/manageiq/manageiq/app/models/job/state_machine.rb:48:in `signal'
/home/agrare/src/manageiq/manageiq/app/models/miq_queue.rb:480:in `block in dispatch_metho

@miq-bot
Copy link
Member

miq-bot commented Jan 6, 2020

Checked commits chessbyte/manageiq@0309004~...cccf7a8 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 16 offenses detected

app/models/vm_scan.rb

@agrare agrare changed the title [WIP] Simplified VmScan base class as all the provider logic has moved into relevant subclasses Simplified VmScan base class as all the provider logic has moved into relevant subclasses Jan 6, 2020
@agrare
Copy link
Member

agrare commented Jan 6, 2020

I pushed a couple of fixes and this now passes for VMware. Lets get this patch set in and follow up with any fixes for other providers as we are able to test them.

@miq-bot miq-bot removed the wip label Jan 6, 2020
@agrare agrare merged commit 3e2b01f into ManageIQ:master Jan 6, 2020
@agrare agrare self-assigned this Jan 7, 2020
@agrare agrare added this to the Sprint 127 Ending Jan 6, 2020 milestone Jan 7, 2020
@chessbyte chessbyte deleted the subclass_vm_scan branch July 9, 2020 15:25
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.

4 participants