-
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
get rid of the activity stream middleware #6525
Conversation
user = User.objects.filter(id=user.id) | ||
if user.exists(): | ||
user = user[0] | ||
instance.actor = user |
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
@@ -363,6 +364,18 @@ def model_serializer_mapping(): | |||
} | |||
|
|||
|
|||
def emit_activity_stream_change(instance): |
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.
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 comment
The 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 awx-manage
command).
Build succeeded.
|
@@ -399,6 +412,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 comment
The 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.
it has bugs and is very confusing see: https://github.com/ansible/tower/issues/4037
7125495
to
9fe2211
Compare
Build succeeded.
|
Build failed (gate pipeline). For information on how to proceed, see
|
regate |
Build failed (gate pipeline). For information on how to proceed, see
|
regate |
Build failed (gate pipeline). For information on how to proceed, see
|
regate |
Build failed (gate pipeline). For information on how to proceed, see
|
regate |
Build failed (gate pipeline). For information on how to proceed, see
|
regate |
Build failed (gate pipeline). For information on how to proceed, see
|
regate |
Build failed (gate pipeline). For information on how to proceed, see
|
regate |
Build succeeded (gate pipeline).
|
it has bugs and is very confusing