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

Fix/privatisation quickie interaction #1094

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

alexanderleegs
Copy link
Contributor

@alexanderleegs alexanderleegs commented Jan 4, 2024

This PR fixes an issue with the integration of privatisation with quickie. To be reviewed in conjunction with PR#1763 on the frontend repo. There are two existing issues being tackled by this PR:

  • Repos which are being made private/unprivate should also affect their respective staging-lite sites, otherwise a site might still contain a publicly accessible version in staging-lite
  • Our DB automatically self corrects to switch any site which has been placed on quickie via growthbook to the staging-lite. variant, but this should not be true for private sites

Note that this PR does not handle the issue of inconsistent states fully right now - it is possible that a user who attempts to privatise/unprivatise a site will have the request to the second amplify site fail, leaving us in an inconsistent state - this needs to be manually resolved for now, as the resolution is complicated. We can look into making this more robust if we find that this is a consistent issue, but I am opting to leave this as is for now and pipe alarms that notify us instead. The issue is also fixable on the user's end if they try again - the inconsistent state will not block cms operations.

TODOs:

  • Add cloudwatch alarm for inconsistent state resolution
  • Add entry in runbook for handling inconsistent state

Tests

  • Privatise a site
    • Ensure that the staging and staging lite sites on amplify get updated with the password
  • Deprivatise a site
    • Ensure that the staging and staging-lite sites on amplify have removed password

Copy link
Contributor

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

lgtm with one question

Comment on lines +268 to +270
logger.error(
`Privatisation adjustment failed for ${repoName} - requires manual fixing of inconsistent state`
)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have an alarm for this? otherwise, we won't know when this occurs until someone feedsback to us, rendering this log moot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexanderleegs alexanderleegs merged commit 42118c5 into develop Jan 17, 2024
17 checks passed
@mergify mergify bot deleted the fix/privatisation-quickie-interaction branch January 17, 2024 08:16
@alexanderleegs alexanderleegs mentioned this pull request Jan 17, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants