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

Ansible Tower Event Catcher #13423

Merged
merged 4 commits into from
Jan 23, 2017
Merged

Conversation

durandom
Copy link
Member

First version of event catcher for ansible tower provider

still open and for a follow up PR:

  • should some events / activity_stream items be skipped?
  • what events should make it into timelines?
  • what events should trigger a refresh?
  • what inventory items can we link to events?

@blomquisg @jameswnl @juliancheal please review

@durandom
Copy link
Member Author

@miq-bot add_labels enhancement, providers/ansible_tower, euwe/no

@durandom
Copy link
Member Author

Oh, and this requires ansible/ansible_tower_client_ruby#54 to be merged and released

@chessbyte chessbyte changed the title Ansible event catcher Ansible Tower Event Catcher Jan 10, 2017
def initialize(ems)
@ems = ems
@last_activity = nil
@stop_polling = false
Copy link
Member

Choose a reason for hiding this comment

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

why the negative name? why not @polling or @polling_enabled

Copy link
Member Author

Choose a reason for hiding this comment

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

because I want to make this read positive

throw :stop_polling if @stop_polling

vs

throw :stop_polling unless @polling_enabled

Copy link
Member

Choose a reason for hiding this comment

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

I think I'm with @chessbyte on this one ... The truthy name and boolean value seems to make more sense. Even with throw :stop_polling unless @polling_enabled

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, convinced 😄 Thanks for your input.

catch(:stop_polling) do
begin
loop do
ap filter
Copy link
Member

Choose a reason for hiding this comment

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

What does the ap method do?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, it's a debugging leftover

@durandom
Copy link
Member Author

@blomquisg pls re-review

also have a look at those event names and where I put them

@miq-bot
Copy link
Member

miq-bot commented Jan 13, 2017

Checked commits durandom/manageiq@e938b1d~...9e8622c with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
5 files checked, 3 offenses detected

app/models/manageiq/providers/ansible_tower/configuration_manager/event_catcher.rb

app/models/manageiq/providers/ansible_tower/configuration_manager/event_catcher/runner.rb

@blomquisg blomquisg merged commit 000f511 into ManageIQ:master Jan 23, 2017
@blomquisg blomquisg added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 23, 2017
@durandom durandom deleted the ansible_event_catcher branch March 17, 2017 18:14
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