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

Proposed to remove a confusing help note about Doctrine #56

Closed

Conversation

javiereguiluz
Copy link
Member

I originally proposed to add this help note:

# Set "serverVersion" to your server version to avoid edge-case exceptions
and extra database calls

Now I propose to remove it because it's confusing:

  • It doesn't truly explain the consequences: "edge cases" and "database calls" sounds vague.
  • The proposed solution ("set the serverVersion") is confusing: which server version? how do I get that? how do I set up that option?

I think this option requires a lengthy explanation to be really understood, so maybe we should leave that for Symfony Docs.

@Pierstoval
Copy link
Contributor

Pierstoval commented May 4, 2017

Then leave that to Symfony docs and add a link to the docs in this recipe, because it's important to have server version.

The server version is important because else Doctrine performs a query to check server version and adapt container compilation depending on the server version.

Check Doctrine/DBAL/Platforms directory and see the different classes that can depend on the platform's version.

@weaverryan
Copy link
Member

Let's include just one URL where the whole string - including serverVersion is explained... likely a symfony docs page as you said.

And if we don't have it already, what about a post install message that says to update the settings in this file?

@craigh
Copy link

craigh commented May 18, 2017

I believe the need for this is removed in the next version of Doctrine?
refs doctrine/dbal#990 and doctrine/dbal#2671

@weaverryan
Copy link
Member

Maybe :). Is the extra query to determine the version only run once when the container is being built? If so, paas will have a problem - they'll build the container before the db is ready. Or, is the extra query on every request? That's kind of lame, but more of a perf optimization. Or, does Doctrine run the query once at runtime and then cache it after? If that's the case, I don't see any reason to keep this.

If someone has a few minutes to run down those answers, that would help (I'm on my phone!)

@Pierstoval
Copy link
Contributor

The extra query is run only on container compilation to determine which driver/platform will be used by Doctrine IIRC

@weaverryan
Copy link
Member

@Pierstoval If it's run on container compilation, then the serverVersion is probably needed... as you may be building the cache when you don't yet have access to the database. platform.sh is like this and I'm guessing that Heroku is as well. What I mean is: without the serverVersion, if the query is made at compile time, deploys would fail if you don't have database access at the time of building your container.

@xabbuh
Copy link
Member

xabbuh commented May 25, 2017

The need for the option should be solved in Doctrine DBAL (see the linked issues above).

@teohhanhui
Copy link
Contributor

teohhanhui commented May 31, 2017

No, the changes in DBAL does not remove the need for the serverVersion parameter. What it provides is a second guess attempt.

@javiereguiluz
Copy link
Member Author

Closing because this was reworded as part of #258.

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.

7 participants