Repository navigation
Resolve service URLs from explicit config; deprecate HOSTNAME - #53
Merged
Merged
Conversation
…STNAME (issue #52) Service base URLs are now resolved per service in this order: 1. Explicit URL config: CAMPUS_AUTH_URL / CAMPUS_API_URL env vars (mirrors campus-cli's CAMPUS_AUTH_URL / auth_url config key). 2. ENV/CAMPUS_ENV defaults: development Railway deployments, staging (campus.nyjc.dev) and production (campus.nyjc.app). The DEPLOY-suffix and testing branches that built https://{HOSTNAME} URLs are removed: HOSTNAME is a container hostname, not a routable domain, and the f-string dropped the port. Deployments relying on those settings now get a DeprecationWarning and the ENV-based default instead; the warning path will be removed once downstream consumers (nyxchange-timetable-v2, campus-suite) set explicit URL config. 14 unit tests pin the resolution contract; README documents the env vars and resolution order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the mitigation from #52: base URLs for the
authandapiservices are no longer derived fromenv.HOSTNAME.New resolution order (per service,
campus_python/__init__.py):CAMPUS_AUTH_URL/CAMPUS_API_URLenv vars — mirrors campus-cli@7b2e895 (CAMPUS_AUTH_URL/auth_urlconfig key).ENV/CAMPUS_ENVdefaults:development→ Railway dev deployments (default),staging→*.campus.nyjc.dev,production→*.campus.nyjc.app.Deprecation transition (per the issue): the
DEPLOY-suffix andtestingbranches that producedhttps://{HOSTNAME}are removed. When a deployment still presents those settings (DEPLOYending in.auth/.api, orENV/CAMPUS_ENV=testing) without explicit URL config, aDeprecationWarningis emitted and the ENV-based default applies. The warning path itself can be dropped in a follow-up once downstream has migrated.Tests
tests/unit/test_base_url_resolution.pypin the contract: explicit config precedence (incl. overENV=production), ENV/CAMPUS_ENVdefaults,DeprecationWarningfor legacy settings, no warning when explicit config is set, invalidENV→ValueError.Docs
CAMPUS_AUTH_URL/CAMPUS_API_URL/ENVadded to the env var table, deprecation note referencing Deprecate env.HOSTNAME-based base URL resolution in campus_python client #52.Downstream coordination
Per the issue audit,
nyxchange-timetable-v2(pins frommain) andcampus-suite(weekly) install this client. Deployments hitting non-default endpoints (local testing, in-service self-calls viaDEPLOY) must setCAMPUS_AUTH_URL/CAMPUS_API_URL— the deprecation warning names the exact var to set.Fixes #52