Skip to content

ref(flags): Move 2 should-be-permanent flags to permanent.py #114934

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.

Sign up for GitHub

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

Open
wedamija wants to merge 2 commits into
base: master
Choose a base branch
from
+17 −12
Open

ref(flags): Move 2 should-be-permanent flags to permanent.py #114934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d23f6c
ref(flags): Move 2 should-be-permanent flags to permanent.py
wedamija May 4, 2026
539e632
ref(features): Add permanent_system_features dict
wedamija May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions src/sentry/features/permanent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@
"organizations:integrations-scm-multi-org": True,
# Enable issue view endpoints and UI
"organizations:issue-views": False,
# Display profile durations on the stats page
"organizations:continuous-profiling-stats": False,
}

permanent_project_features = {
Expand All @@ -139,6 +141,14 @@
"organizations:workflow-engine-log-evaluations": False,
}

# Flagpole cannot control system-scoped flags — keep these as INTERNAL.
permanent_system_features = {
# Enables user registration.
"auth:register": True,
# Enable support for multiple regions, and org slug subdomains (customer-domains).
"system:multi-region": False,
}

for org_feature, default in permanent_organization_features.items():
manager.add(
org_feature,
Expand Down Expand Up @@ -166,11 +176,10 @@
api_expose=False,
)

# Enable support for multiple regions, and org slug subdomains (customer-domains).
manager.add(
"system:multi-region",
SystemFeature,
FeatureHandlerStrategy.INTERNAL,
default=False,
api_expose=False,
)
for system_feature, default in permanent_system_features.items():
manager.add(
system_feature,
SystemFeature,
FeatureHandlerStrategy.INTERNAL,
default=default,
)

Check warning on line 185 in src/sentry/features/permanent.py

View check run for this annotation

@sentry/warden / warden: sentry-backend-bugs

auth:register system feature loses explicit api_expose=False 

The previous registration of system:multi-region passed api_expose=False explicitly. The new loop registers both auth:register and system:multi-region without passing api_expose, so both will use the manager.add default. If the default for api_expose is True (or differs from False), this silently exposes auth:register and system:multi-region via the API, which is a behavior change despite the PR claiming none. Verify FeatureManager.add's api_expose default before merging.
Comment on lines +179 to +185
Copy link
Copy Markdown
Contributor

@sentry-warden sentry-warden Bot May 5, 2026

Choose a reason for hiding this comment

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

auth:register system feature loses explicit api_expose=False

The previous registration of system:multi-region passed api_expose=False explicitly. The new loop registers both auth:register and system:multi-region without passing api_expose, so both will use the manager.add default. If the default for api_expose is True (or differs from False), this silently exposes auth:register and system:multi-region via the API, which is a behavior change despite the PR claiming none. Verify FeatureManager.add's api_expose default before merging.

Verification

Read the diff: original system:multi-region registration explicitly set api_expose=False, while the new loop omits api_expose. Did not read FeatureManager.add to confirm its default value, hence medium confidence. This falls under Check 11 (logic correctness) — a behavior-changing omission in a 'pure registration move' PR.

Identified by Warden sentry-backend-bugs · WXT-XCJ

4 changes: 0 additions & 4 deletions src/sentry/features/temporary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ def register_temporary_features(manager: FeatureManager) -> None:
# You won't be able to control system feature flags with flagpole, as flagpole only handles
# organization or project scoped features. You would need to use an option instead.

# Enables user registration.
manager.add("auth:register", SystemFeature, FeatureHandlerStrategy.INTERNAL, default=True)
# Enable creating organizations within sentry (if SENTRY_SINGLE_ORGANIZATION is not enabled).
manager.add("organizations:create", SystemFeature, FeatureHandlerStrategy.INTERNAL, default=True)
# Controls whether or not the relocation endpoints can be used.
Expand All @@ -64,8 +62,6 @@ def register_temporary_features(manager: FeatureManager) -> None:
manager.add("organizations:code-review-experiments-enabled", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Enable continuous profiling
manager.add("organizations:continuous-profiling", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Display profile durations on the stats page
manager.add("organizations:continuous-profiling-stats", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=True)
# Enable the ingestion of profile functions metrics into EAP
manager.add("projects:profile-functions-metrics-eap-ingestion", ProjectFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Enables read only dashboards
Expand Down
Loading