-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
get rid of the activity stream middleware #6525
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,7 @@ | |
__all__ = [] | ||
|
||
logger = logging.getLogger('awx.main.signals') | ||
analytics_logger = logging.getLogger('awx.analytics.activity_stream') | ||
|
||
# Update has_active_failures for inventory/groups when a Host/Group is deleted, | ||
# when a Host-Group or Group-Group relationship is updated, or when a Job is deleted | ||
|
@@ -363,6 +364,22 @@ def model_serializer_mapping(): | |
} | ||
|
||
|
||
def emit_activity_stream_change(instance): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any time we trigger a signal for Activity Stream create/update/delete/associate, just schedule a post-commit function that notifies the external logger. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has the nice side effect of actually making external logging of the activity stream work outside of HTTP requests (like if you call an |
||
if 'migrate' in sys.argv: | ||
# don't emit activity stream external logs during migrations, it | ||
# could be really noisy | ||
return | ||
from awx.api.serializers import ActivityStreamSerializer | ||
actor = None | ||
if instance.actor: | ||
actor = instance.actor.username | ||
summary_fields = ActivityStreamSerializer(instance).get_summary_fields(instance) | ||
analytics_logger.info('Activity Stream update entry for %s' % str(instance.object1), | ||
extra=dict(changes=instance.changes, relationship=instance.object_relationship_type, | ||
actor=actor, operation=instance.operation, | ||
object1=instance.object1, object2=instance.object2, summary_fields=summary_fields)) | ||
|
||
|
||
def activity_stream_create(sender, instance, created, **kwargs): | ||
if created and activity_stream_enabled: | ||
# TODO: remove deprecated_group conditional in 3.3 | ||
|
@@ -399,6 +416,9 @@ def activity_stream_create(sender, instance, created, **kwargs): | |
else: | ||
activity_entry.setting = conf_to_dict(instance) | ||
activity_entry.save() | ||
connection.on_commit( | ||
lambda: emit_activity_stream_change(activity_entry) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only thing that concerns me is that this will now be ran in migrations. If some migration created 1M activity stream entries, these methods would pile up, and that would cause problems. I don't feel like migrations should create any activity stream entries, and I disable them when the issue comes up. |
||
) | ||
|
||
|
||
def activity_stream_update(sender, instance, **kwargs): | ||
|
@@ -430,6 +450,9 @@ def activity_stream_update(sender, instance, **kwargs): | |
else: | ||
activity_entry.setting = conf_to_dict(instance) | ||
activity_entry.save() | ||
connection.on_commit( | ||
lambda: emit_activity_stream_change(activity_entry) | ||
) | ||
|
||
|
||
def activity_stream_delete(sender, instance, **kwargs): | ||
|
@@ -467,6 +490,9 @@ def activity_stream_delete(sender, instance, **kwargs): | |
object1=object1, | ||
actor=get_current_user_or_none()) | ||
activity_entry.save() | ||
connection.on_commit( | ||
lambda: emit_activity_stream_change(activity_entry) | ||
) | ||
|
||
|
||
def activity_stream_associate(sender, instance, **kwargs): | ||
|
@@ -540,6 +566,9 @@ def activity_stream_associate(sender, instance, **kwargs): | |
activity_entry.role.add(role) | ||
activity_entry.object_relationship_type = obj_rel | ||
activity_entry.save() | ||
connection.on_commit( | ||
lambda: emit_activity_stream_change(activity_entry) | ||
) | ||
|
||
|
||
@receiver(current_user_getter) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need to set the actor in this middleware; it's already set in the various signals that create these objects:
https://github.com/ansible/awx/blob/devel/awx/main/signals.py#L392
https://github.com/ansible/awx/blob/devel/awx/main/signals.py#L426
https://github.com/ansible/awx/blob/devel/awx/main/signals.py#L468
https://github.com/ansible/awx/blob/devel/awx/main/signals.py#L522