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

Refactor plugin sync #1200

Merged
merged 10 commits into from
Jan 25, 2023
Merged

Refactor plugin sync #1200

merged 10 commits into from
Jan 25, 2023

Conversation

iskhakov
Copy link
Contributor

@iskhakov iskhakov commented Jan 24, 2023

What this PR does

This PR adds a shortcut in the plugin synchronisation process, so the existing users will be able login without waiting for the sync task. Every request still starts the background synchronisation task, to be able to propagate the organisation changes faster than periodic task. It means that we don't necessarily need "force reload" button in the interface.
For all the other cases (user does not exist, organisation token "not ok", etc) process remains same - plugin will show "Initialising plugin..." until the background task in successfully completed

Which issue(s) this PR fixes

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated

@iskhakov iskhakov requested a review from a team January 24, 2023 05:24
engine/apps/auth_token/auth.py Outdated Show resolved Hide resolved
organization.api_token_status = Organization.API_TOKEN_STATUS_PENDING

organization.save(update_fields=["api_token_status"])
try:

Choose a reason for hiding this comment

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

Could you please add comments here? I didn't catch the meaning of that check and why we setting API_TOKEN_STATUS_PENDING if we failed to get user from grafana context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For all the other cases (user does not exist, organisation token "not ok", etc) process remains same - plugin will show "Initialising plugin..." until the background task in successfully completed

Choose a reason for hiding this comment

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

Yep, but what is the case you are checking here and what is the expected behaviour? I mean, in which case we are unable to get user from grafana context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw, cleaned up a little and added pr description
you were too fast:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed is_installed part, thanks

@iskhakov iskhakov added the pr:no public docs Added to a PR that does not require public documentation updates label Jan 24, 2023
Copy link
Contributor

@mderynck mderynck left a comment

Choose a reason for hiding this comment

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

LGTM
We can further optimize on UI side if we add the token_ok, license and version from the GET call to the data for the POST call. But that should probably be a different PR, this accomplishes most of the speedup while not needing to change UI.

@iskhakov iskhakov merged commit 1fc3f6d into dev Jan 25, 2023
@iskhakov iskhakov deleted the iskhakov/refactor-pluginsync branch January 25, 2023 01:12
@iskhakov
Copy link
Contributor Author

Fixes part of this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants