Skip to content
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

Remove charset config for doctrine-bundle v2 #693

Merged
1 commit merged into from Nov 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
Expand Up @@ -2,12 +2,11 @@ doctrine:
dbal:
url: '%env(resolve:DATABASE_URL)%'

# IMPORTANT: You MUST configure your db driver and server version,
# IMPORTANT: You MUST configure your server version,
# either here or in the DATABASE_URL env var (see .env file)
#driver: 'mysql'
#server_version: '5.7'

# Only needed for MySQL (ignored otherwise)
# only needed for MySQL
charset: utf8mb4
default_table_options:
collate: utf8mb4_unicode_ci
Expand Down
5 changes: 2 additions & 3 deletions doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml
Expand Up @@ -2,12 +2,11 @@ doctrine:
dbal:
url: '%env(resolve:DATABASE_URL)%'

# IMPORTANT: You MUST configure your db driver and server version,
# IMPORTANT: You MUST configure your server version,
# either here or in the DATABASE_URL env var (see .env file)
#driver: 'mysql'
#server_version: '5.7'

# Only needed for MySQL (ignored otherwise)
# only needed for MySQL
charset: utf8mb4
default_table_options:
collate: utf8mb4_unicode_ci
Expand Down
4 changes: 2 additions & 2 deletions doctrine/doctrine-bundle/1.6/manifest.json
Expand Up @@ -9,8 +9,8 @@
"env": {
"#1": "Format described at https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html#connecting-using-a-url",
"#2": "For an SQLite database, use: \"sqlite:///%kernel.project_dir%/var/data.db\"",
"#3": "For a PostgreSQL database, use: \"postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=11&charset=UTF-8\"",
"#4": "IMPORTANT: You MUST configure your db driver and server version, either here or in config/packages/doctrine.yaml",
"#3": "For a PostgreSQL database, use: \"postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=11&charset=utf8\"",
"#4": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
"DATABASE_URL": "mysql://db_user:db_password@127.0.0.1:3306/db_name?serverVersion=5.7"
}
}
18 changes: 18 additions & 0 deletions doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
@@ -0,0 +1,18 @@
doctrine:
dbal:
url: '%env(resolve:DATABASE_URL)%'

# IMPORTANT: You MUST configure your server version,
# either here or in the DATABASE_URL env var (see .env file)
#server_version: '5.7'
Copy link
Member

Choose a reason for hiding this comment

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

What about removing everything here? Or put another way: why would one want to configure the driver and version here instead of in the DATABASE_URL env var?

Copy link
Member

Choose a reason for hiding this comment

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

Also, the DSN are like: mysql://..., postgresql://..., sqlite://... So, why do I have to define the driver explicitly? It's just the first word of the DSN, right?

Copy link
Member

Choose a reason for hiding this comment

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

you might want to configure the version here in case your PaaS does not expose a serverVersion parameter in the query string (which is a Doctrine DBAL convention). This happens on Heroku for instance.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dzuelke WDYT about this? Could Heroku include the serverVersion in the DATABASE_URL?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the "driver" line but kept "server_version" for now.

Copy link

Choose a reason for hiding this comment

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

We can't, since we don't offer MySQL, @nicolas-grekas ;) That's probably a question for the folks at ClearDB and JawsDB. I generally think that the server version is more of a "static" environment thing, since your code is probably also written against the specific capabilities of a version series, so it shouldn't differ between environments, unlike the DSN.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not only for MySQL - the same is needed for PG :)
But I get your point. Actually, DoctrineBundle/DBAL could throw an exception when both settings are found but don't match (in DSN & config). That would be even more useful as it would help spot mistakes.

/cc @alcaeus in case you agree and want to create an issue there :)

Copy link
Contributor

Choose a reason for hiding this comment

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

orm:
auto_generate_proxy_classes: true
naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
auto_mapping: true
mappings:
App:
is_bundle: false
type: annotation
dir: '%kernel.project_dir%/src/Entity'
prefix: 'App\Entity'
alias: App
1 change: 1 addition & 0 deletions doctrine/doctrine-bundle/2.0/manifest.json
1 change: 1 addition & 0 deletions doctrine/doctrine-bundle/2.0/post-install.txt
1 change: 1 addition & 0 deletions doctrine/doctrine-bundle/2.0/src