Skip to content

Conversation

@rragundez
Copy link
Contributor

It is common in application to need the host, for example for redirect callbacks using oauth, or for automated CORS restrictions, etc.

This PR adds the host settings and introduces a pattern for settings validation based on environments. Within this pattern validation it adds helpful checks over CORS and the definition of hosts for staging and production.

It is important to show that the pattern also does not fix things for the user or performs changes to their settings based on other settings, i have always found that is better to give the feedback to the user and let the user decide instead of performing modifications at runtime without visibility to the user. That is why in the validation there should only be messages warnings/errors and no modification of the settings.

@rragundez rragundez marked this pull request as ready for review November 21, 2025 07:32
@igorbenav igorbenav changed the base branch from main to staging November 21, 2025 18:49
@rragundez
Copy link
Contributor Author

@LucasQR @igorbenav

@igorbenav
Copy link
Collaborator

Really awesome but It's missing an update all .env.example files.

The new host settings (APP_BACKEND_HOST and APP_FRONTEND_HOST) are only documented in scripts/local_with_uvicorn/.env.example. They should be added to the other environment examples (scripts/gunicorn_managing_uvicorn_workers/.env.example and scripts/production_with_nginx/.env.example) with appropriate default values:

  • Local/Staging: APP_BACKEND_HOST="http://localhost:8000", APP_FRONTEND_HOST="http://localhost:3000"
  • Production: APP_BACKEND_HOST="https://api.example.com", APP_FRONTEND_HOST="https://example.com"

@rragundez another comment to address before merging staging

@igorbenav igorbenav merged commit 53d7a83 into benavlabs:staging Nov 24, 2025
3 checks passed
@rragundez rragundez deleted the add-hosts branch November 25, 2025 07:02
@rragundez
Copy link
Contributor Author

Hi @igorbenav thanks for the feedback, i did not add them to the other ones because I think at the end they will be deleted. As mentioned in one discussion/issue I do not think there should be different env files nor the concept of environments. There should be functionalities regardless of the environment and only in the documentation perhaps mention best practices, as I mention in the related issue/discussion this will uncluttered the documentation and code from trying to guess what the user wants to do in each environment.
We can discuss this further once I put the PR to remove these other .envs files, for the moment I just use the local one as the source of truth

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