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

Address event correlation issues in the awx_display callback #1345

Draft
wants to merge 6 commits into
base: devel
Choose a base branch
from

Conversation

sivel
Copy link
Member

@sivel sivel commented Mar 13, 2024

No description provided.

@@ -311,7 +295,15 @@ def wrapper(*args, **kwargs):
return wrapper


Display.display = display_with_context(Display.display)
for _display_attr in ('banner', 'deprecated', 'display', 'error',
'system_warning', 'verbose', 'warning'):
Copy link
Member Author

Choose a reason for hiding this comment

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

The display events "supported" by awx are only:

        (0, 'debug', _('Debug'), False),
        (0, 'verbose', _('Verbose'), False),
        (0, 'deprecated', _('Deprecated'), False),
        (0, 'warning', _('Warning'), False),
        (0, 'system_warning', _('System Warning'), False),
        (0, 'error', _('Error'), True),

This would potentially apply to unclassified_display too. Although the previous monkeypatching didn't exclude banner.

@@ -386,14 +386,14 @@ def _emit_event(self,
event_data = self._current_event_data
stdout_chunks = [buffered_stdout]
elif buffered_stdout:
event_data = dict({'event': 'verbose'})
event_data = dict({'event': 'unclassified_display'})
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

All of the CI failures appear to be related to this, where verbose is being excluded, or depended on. I'm not sure we can make this change, but I'd prefer to not just classify any unknown display event as verbose if possible.

C.config.initialize_plugin_configuration_definitions(
'callback', self._load_name, base_config | my_config
)
return super().set_options(*args, **kwargs)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not related, but since I'm in here making a mess, this fixes the options issue with callbacks not matching the default from a config perspective.

Copy link

sonarcloud bot commented Jul 25, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant