Skip to content

shorter example for env resolve (2) #18128

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

Conversation

alexislefebvre
Copy link
Contributor

This is a follow up of #18107

I took the recipe from https://github.com/symfony/recipes/tree/main/lexik/jwt-authentication-bundle/2.5 as an example.

There are pros and cons, so I explain them to help contributors to decide.

Pros:

  • shorter example
  • more relevant IMHO, using different Sentry hosts looked confusing to me: Sentry handle several projects on one instance (so with one IP address)

Cons:

  • less consistent with the other examples

@carsonbot carsonbot added this to the 5.4 milestone Mar 27, 2023
env(SENTRY_DSN): 'http://%sentry_host%/project'
sentry:
dsn: '%env(resolve:SENTRY_DSN)%'
env(JWT_SECRET_KEY): '%kernel.project_dir%/%CONFIG_DIR%/jwt/private.pem'
Copy link
Member

@javiereguiluz javiereguiluz Jul 12, 2023

Choose a reason for hiding this comment

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

What I don't like in this example is that it's not clear where %CONFIG_DIR% comes from. In the previous example we had the sentry_host parameter and then it was used in the other parameter.

Maybe we can do the following?

parameters:
    jwt_config_dir: 'config/jwt'
    env(JWT_SECRET_KEY): '%kernel.project_dir%/%jwt_config_dir%/private.pem'

lexik_jwt_authentication:
    secret_key: '%env(resolve:JWT_SECRET_KEY)%'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's better with your suggestion. But the example is not shorter than before.

On second thought, the current example with Sentry is fine, this doc is about explaining how env vars work, not about Sentry.

I suggest that we close this PR and keep the examples as they are.

@alexislefebvre alexislefebvre force-pushed the shorter-example-for-env-resolve branch from 73ed876 to d6960e0 Compare July 13, 2023 09:39
@OskarStark OskarStark closed this Jul 25, 2023
@alexislefebvre alexislefebvre deleted the shorter-example-for-env-resolve branch July 25, 2023 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants