Skip to content

Conversation

@max-moser
Copy link
Contributor

@max-moser max-moser commented Oct 28, 2022

This can be used to implement a conditional read-only mode, e.g. enabled temporarily to make system migrations smoother.

To do:

  • add some tests for the new generators and configuration items

@max-moser max-moser force-pushed the mm/conditional-disable-generator branch from 45e082e to b250e67 Compare February 14, 2023 15:56
@tmorrell tmorrell requested a review from ntarocco February 16, 2023 17:46
@fenekku fenekku requested a review from utnapischtim March 10, 2023 13:40
@fenekku
Copy link
Contributor

fenekku commented Mar 10, 2023

Coordination:

This one has been sitting for 3weeks+

Surfacing this back to attention for @utnapischtim , @ppanero and @ntarocco . If someone else would be better suited to look at it, let us know!

Copy link
Contributor

@ntarocco ntarocco left a comment

Choose a reason for hiding this comment

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

LGTM! It would be even better to add a test for a service:

RECORDS_PERMISSIONS_READ_ONLY = True

with PermissionDenied:
    service.update(record, "owner", data)

@chokribr
Copy link
Member

@utnapischtim @ppanero please take a look!

@max-moser max-moser force-pushed the mm/conditional-disable-generator branch from b250e67 to 3f9cc1a Compare March 19, 2024 15:16
* it seems like it was missing before
* this can be used to implement a conditional read-only mode, e.g.
  enabled temporarily to make system migrations smoother
@max-moser max-moser force-pushed the mm/conditional-disable-generator branch from 3f9cc1a to a6c3afa Compare March 20, 2024 15:23
@max-moser
Copy link
Contributor Author

Given that Mr. E already provided the ConditionalGenerator, I reworked this PR to be based off that instead.
Over the course of that, the functionality of having the config option a callable got thrown out the window.

Given that we override most of the permission policies in our own modules anyway, we can add that feature in locally for us.

@ntarocco ntarocco removed this from PR Community Mar 26, 2025
@ntarocco ntarocco added this to vNext Mar 26, 2025
@ntarocco ntarocco moved this to Triage in vNext Mar 26, 2025
@utnapischtim utnapischtim moved this from Triage to 👀 In review in vNext May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

5 participants