Skip to content

Define a constant instead of duplicating 'http://%s' (Sonar S1192) - #565

Merged
mzeier merged 1 commit into
stage-eksfrom
449/settings-base-s1192
Oct 12, 2026
Merged

mzeier merged 1 commit into
stage-eksfrom
449/settings-base-s1192

Conversation

@mzeier

@mzeier mzeier commented Oct 12, 2026

Copy link
Copy Markdown

Summary

Fixes python:S1192 on src/olympia/lib/settings_base.py:229 ("Define a constant instead of duplicating this literal 'http://%s' 3 times", Sonar issue key AaEh92ixMSBE_HpBvX1E).

Introduces a single module-level _http_url_format = 'http://%s' constant and uses it at all three duplicate sites (lines 229, 237, 1524). The name is lowercase with a leading underscore so Django's settings machinery does not pick it up as a setting (uppercase names in this module become settings and show up in zadmin's settings page).

Equivalence

No behavior change. Each of the three expressions evaluates to the byte-identical value as before:

  • SITE_URL = _http_url_format % DOMAIN == 'http://%s' % DOMAIN
  • SERVICES_URL = _http_url_format % SERVICES_DOMAIN == 'http://%s' % SERVICES_DOMAIN
  • ES_URLS = [_http_url_format % h for h in ES_HOSTS] == ['http://%s' % h for h in ES_HOSTS]

_http_url_format holds the exact same string literal used at each site, so the % formatting operands and results are unchanged.

Context

python:S1192 on this file was explicitly declined for exclusion in the thunderbird/addons-server#449 register ("a no-behavior-change fix exists"). The companion python:S5332 ("Using HTTP protocol is insecure") new-code flag this refactor would otherwise trigger is covered by an accepted exclusion in that same register (python:S5332 / src/olympia/lib/settings_base.py, 4 occurrences), applied to the project today.

No other change in the file.

settings_base.py repeated the 'http://%s' format string at SITE_URL,
SERVICES_URL and ES_URLS. Introduce one module-level _http_url_format
constant and use it at all three sites with identical % formatting;
the lowercase leading-underscore name keeps Django from exposing it
as a setting. No behavior change: each of the three expressions
evaluates to the exact same value as before.
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

Python Test Results

5 763 tests  ±0   5 709 ✅ ±0   9m 36s ⏱️ - 1m 54s
    1 suites ±0      54 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit c8e4137. ± Comparison against base commit 0dd650e.

@mzeier
mzeier merged commit 10391c3 into stage-eks Oct 12, 2026
3 checks passed
@mzeier
mzeier deleted the 449/settings-base-s1192 branch October 12, 2026 16:12
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.

1 participant