-
Notifications
You must be signed in to change notification settings - Fork 489
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
Conditional creation of case resources for perf improvement on signal ingest #3331
Conversation
src/dispatch/case/flows.py
Outdated
document=document, project_id=case.project.id, db_session=db_session | ||
) | ||
if create_resources: | ||
case_create_resources_flow(db_session=db_session, case_id=case.id) |
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.
Should we move the code for creating a conversation (that doesn't leverage the functions in the conversation.flows
module) inside case_create_resources_flow
?
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.
I was wondering that but I wasn't sure. It's core to the workflow and if we moved it to case_create_resources_flow
it would be too binary. We have the conversation_target
flag in the Signal, which is our conditional for creating a conversation or not.
if conversation_target:
try:
# TODO: Refactor conversation creation using conversation_flows module
conversation = create_conversation(case, conversation_target, db_session)
conversation_in = ConversationCreate(
Also, we should probably use convo flow here or create a new flow for this.
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.
I thought the plan was to not create new conversations, but leveraging existing and configured ones in Dispatch for sending notifications and threading them. What am I missing?
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.
If we still need to create a conversation and this depends on individual_participants
and team_participants
, should these be passed as parameters to case_create_resources_flow
to avoid fetching them twice?
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.
I thought the plan was to not create new conversations, but leveraging existing and configured ones in Dispatch for sending notifications and threading them. What am I missing?
Only net new (non-deduplicated/snooze) Signals should ever enter the case_new_create_flow
.
Filtered ones update the existing thread:
# we don't need to continue if a filter action took place
if signal_service.filter_signal(db_session=db_session, signal_instance=signal_instance):
# If the signal was deduplicated, we can assume a case exists,
# and we need to update the corresponding signal message
if (
signal_instance.filter_action == SignalFilterAction.deduplicate
and signal_instance.case.signal_thread_ts # noqa
):
update_signal_message(
db_session=db_session,
signal_instance=signal_instance,
)
return signal_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.
Updated to pass the participants as args to create resource flow
Resources are still created successfully on case escalation