Skip to content

Explain what happens if flush() fails #6894

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

Merged
merged 2 commits into from
Dec 15, 2016
Merged

Conversation

ThomasLandauer
Copy link
Contributor

It should be mentioned somewhere in this chapter, how to verify that the entity has really been persisted. What's the best practice here? try...catch? Or (for INSERT) check if $entity->getId() does exist? => Please expand on that.
Also, you could list all possible types of exceptions explicitly.

It should be mentioned *somewhere* in this chapter, how to verify that the entity has really been persisted. What's the best practice here? try...catch? Or (for INSERT) check if $entity->getId() does exist? => Please expand on that.
Also, you could list all possible types of exceptions explicitly.
@@ -555,6 +555,11 @@ Take a look at the previous example in more detail:
``Product`` objects and then subsequently call ``flush()``, Doctrine will
execute 100 ``INSERT`` queries using a single prepared statement object.

.. note::

If ``flush()`` fails, an exception is thrown, which you can catch by wrapping
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 replacing this:

If ``flush()`` fails, an exception is thrown, ...

by this:

If ``flush()`` call fails, a ``Doctrine\ORM\ORMException`` exception is thrown, ...

@ThomasLandauer
Copy link
Contributor Author

ThomasLandauer commented Aug 29, 2016

Thanks, I just integrated the Doctrine\ORM\ORMException hint :-)

@xabbuh
Copy link
Member

xabbuh commented Sep 21, 2016

Hm, do we really need this? Especially beginners may think that they always have to do this.

@ThomasLandauer
Copy link
Contributor Author

Well, _I_ was looking for this kind of explicit answer....
In my view, the docs are not a quick how-to for beginners, but also a full documentation for advanced users. Besides, the wording ("if....you can...") doesn't imply that everyone should.

@linaori
Copy link
Contributor

linaori commented Oct 16, 2016

I think you should also note that the entity manager ends up in closed state because it cannot guarantee the state of the managed entities, thus becomes utterly useless. A new instance should be made and all managed entities removed.

tl;dr while catching the exception is good as explanation, it should also document the consequences.

@weaverryan weaverryan merged commit 460b74a into symfony:2.7 Dec 15, 2016
weaverryan added a commit that referenced this pull request Dec 15, 2016
This PR was merged into the 2.7 branch.

Discussion
----------

Explain what happens if `flush()` fails

It should be mentioned _somewhere_ in this chapter, how to verify that the entity has really been persisted. What's the best practice here? try...catch? Or (for INSERT) check if $entity->getId() does exist? => Please expand on that.
Also, you could list all possible types of exceptions explicitly.

Commits
-------

460b74a Integrated improvement by javiereguiluz
f3b07bf Explain what happens if `flush()` fails
weaverryan added a commit that referenced this pull request Dec 15, 2016
@weaverryan
Copy link
Member

Thanks Thomas! I've linked to Doctrine's docs also, it's really their place to get into more details.

Cheers!

weaverryan added a commit that referenced this pull request Dec 15, 2016
* 2.7: (22 commits)
  [#6894] Adding link to Doctrine
  [#6857] Re-wording the section a bit
  [#6840] Adding note based on Stof's feedback
  Moving file - I think configuration is more appropriate
  Revert "Updated the contents for the new Symfony 3 dir structure"
  Updated the contents for the new Symfony 3 dir structure
  Remove "Symfony3 will use"
  Using lower case on "Form"
  Made a bunch of fixes recommended by Ryan
  Added a note about rendering templates from different kernels
  Added a new section about running commands under a different kernel
  Integrated improvement by javiereguiluz
  Explain what happens if `flush()` fails
  Update events.rst
  Added a new use case related to micro-services
  Removed a use case
  Reworded the use cases section
  Fixed typo
  Fixed an example code
  Fixed another syntax issue
  ...
weaverryan added a commit that referenced this pull request Dec 15, 2016
* 2.8: (22 commits)
  [#6894] Adding link to Doctrine
  [#6857] Re-wording the section a bit
  [#6840] Adding note based on Stof's feedback
  Moving file - I think configuration is more appropriate
  Revert "Updated the contents for the new Symfony 3 dir structure"
  Updated the contents for the new Symfony 3 dir structure
  Remove "Symfony3 will use"
  Using lower case on "Form"
  Made a bunch of fixes recommended by Ryan
  Added a note about rendering templates from different kernels
  Added a new section about running commands under a different kernel
  Integrated improvement by javiereguiluz
  Explain what happens if `flush()` fails
  Update events.rst
  Added a new use case related to micro-services
  Removed a use case
  Reworded the use cases section
  Fixed typo
  Fixed an example code
  Fixed another syntax issue
  ...
weaverryan added a commit that referenced this pull request Dec 15, 2016
* 3.1: (25 commits)
  [#6894] Adding link to Doctrine
  [#6857] Re-wording the section a bit
  [#6840] Adding note based on Stof's feedback
  Moving file - I think configuration is more appropriate
  Revert "Updated the contents for the new Symfony 3 dir structure"
  Updated the contents for the new Symfony 3 dir structure
  Remove "Symfony3 will use"
  Using lower case on "Form"
  Make lines shorter to comply with our soft limit of 80 chars per line
  Made a bunch of fixes recommended by Ryan
  Added a note about rendering templates from different kernels
  Fix comments
  [Serializer] Docs for the @MaxDepth annotation
  Added a new section about running commands under a different kernel
  Integrated improvement by javiereguiluz
  Explain what happens if `flush()` fails
  Update events.rst
  Added a new use case related to micro-services
  Removed a use case
  Reworded the use cases section
  ...
weaverryan added a commit that referenced this pull request Dec 15, 2016
* 3.2: (26 commits)
  [#6894] Adding link to Doctrine
  [#6857] Re-wording the section a bit
  [#6840] Adding note based on Stof's feedback
  Moving file - I think configuration is more appropriate
  Revert "Updated the contents for the new Symfony 3 dir structure"
  Updated the contents for the new Symfony 3 dir structure
  Remove "Symfony3 will use"
  Using lower case on "Form"
  Added a mention to sameSite cookie option
  Make lines shorter to comply with our soft limit of 80 chars per line
  Made a bunch of fixes recommended by Ryan
  Added a note about rendering templates from different kernels
  Fix comments
  [Serializer] Docs for the @MaxDepth annotation
  Added a new section about running commands under a different kernel
  Integrated improvement by javiereguiluz
  Explain what happens if `flush()` fails
  Update events.rst
  Added a new use case related to micro-services
  Removed a use case
  ...
@ThomasLandauer ThomasLandauer deleted the patch-10 branch December 15, 2016 16:51
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.

6 participants