-
Notifications
You must be signed in to change notification settings - Fork 987
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
Read only AdminFlag #3393
Read only AdminFlag #3393
Conversation
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 is great! Only one concern, but I'm OK either way.
warehouse/db.py
Outdated
# Check if we're in read-only mode | ||
from warehouse.admin.flags import AdminFlag | ||
flag = session.query(AdminFlag).get('read-only') | ||
if flag and flag.enabled: |
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.
can we get an overall admin user short circuit here as well? that would alleviate the need to except for this in edit_flag
and avoid having to except this for each and every operation an admin may need to perform during read-only situations.
warehouse/admin/views/flags.py
Outdated
flag.description = request.POST['description'] | ||
flag.enabled = bool(request.POST.get('enabled')) | ||
|
||
if flag.id == 'read-only' and request.tm.isDoomed(): |
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.
not sure having to add this clause for every admin action we add down the line is a a great approach. suggest we do this once in warehouse.db
What's the behavior that users are going to see if they attempt to do something that writes to the DB when we're in read only mode? |
@dstufft It will look like the write has succeeded, but it won't, and there will be a big red notification banner saying "Read Only Mode: Any write operations will have no effect" We probably need to add a special case to fail upload since twine users won't see the notification banner. |
Fixes #164.
read-only
admin flag which dooms the transactionrequest.flags.enabled
andrequest.flags.all
)AdminFlag.notify
attribute to determine if the flag should be displayed as a notification bar