-
Notifications
You must be signed in to change notification settings - Fork 275
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
Periodic chatops proxy sync #4565
Conversation
…' into post_stack_slug_to_chatops_proxy
try: | ||
register_oncall_tenant(org) | ||
except ChatopsProxyAPIException as api_exc: | ||
# TODO: once tenants upsert api is released, remove this check |
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.
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.
It's change on chatops-proxy which make tenants/register update tenant with stack_id and cluster slug instead of returning 409 on conflict
SERVICE_TYPE_ONCALL, | ||
stack_id, | ||
) | ||
register_oncall_tenant(org) | ||
except Exception as e: |
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.
down below in the call to register_oncall_tenant_async.apply_async
, aren't you missing the new stack_slug
kwarg to pass in?
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.
Yep, nice catch
engine/apps/chatops_proxy/utils.py
Outdated
def uninstall_slack(stack_id: int, grafana_user_id: int) -> bool: | ||
""" | ||
uninstall_slack uninstalls slack integration from chatops-proxy and returns bool indicating if it was removed. | ||
If such installation does not exist - returns True as well.s |
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 such installation does not exist - returns True as well.s | |
If such installation does not exist - returns True as well. |
engine/apps/chatops_proxy/utils.py
Outdated
) | ||
return False | ||
|
||
return removed is True |
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.
isn't removed
always a boolean? what about returning return removed
?
# Conflicts: # engine/apps/chatops_proxy/tasks.py # engine/apps/chatops_proxy/utils.py # engine/apps/slack/views.py # engine/apps/user_management/models/organization.py
Adds a pediodic job to sync tenants with chatops-proxy. Register request will behave as upsert, allowing us to backfill new tenant's columns: stack_id and stack_slug. Upsert is not merged on chatops-proxy, so that's why task handling 409 status on /tenants/register request.
On top of that, I did small refactoring and introduced a new register_oncall_tenant func, which receives org as an argument to not to write
register_tenant(org.uuid, org.stack_id, org.stack_slug,.....)
every time.Part of https://github.com/grafana/oncall-gateway/issues/247
Need to be merged after #4559.