Skip to content

[Messenger] Disable the SchemaAssetsFilter when setup the transport #31625

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

Conversation

vincenttouzet
Copy link
Contributor

Q A
Branch? 4.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #31623
License MIT
Doc PR symfony/symfony-docs#...

@chalasr
Copy link
Member

chalasr commented May 26, 2019

Tests seem broken

@vincenttouzet
Copy link
Contributor Author

Tests seem broken

Yes I'm on it 😉

@vincenttouzet vincenttouzet force-pushed the 31623_messenger_doctrine_schema_filter branch 3 times, most recently from 638cffd to 1536f42 Compare May 26, 2019 15:12
@vincenttouzet vincenttouzet force-pushed the 31623_messenger_doctrine_schema_filter branch from 1536f42 to 8cbb8f8 Compare May 26, 2019 15:41
@vincenttouzet
Copy link
Contributor Author

Well the fix is uglier than I thought 🤔

As of Doctrine 2.9 the method to configure the assets filter has changed : doctrine/dbal@111e42d

doctrine-dbal 2.9 introduce the new methods getSchemaAssetsFilter / setSchemaAssetsFilter. Before it was getFilterSchemaAssetsExpression / getFilterSchemaAssetsExpression

So I need to try if the method exists before

@weaverryan
Copy link
Member

Quick work - thanks! Yea, that got ugly :). I can't think of another way either - can anyone else? If not, I think we need this - without it, it's an even uglier situation for the user.

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Let's merge as is for 4.3. We will have time to tweak it if possible in 4.4.

@fabpot
Copy link
Member

fabpot commented May 27, 2019

Thank you @vincenttouzet.

@fabpot fabpot merged commit 8cbb8f8 into symfony:4.3 May 27, 2019
fabpot added a commit that referenced this pull request May 27, 2019
…transport (vincenttouzet)

This PR was merged into the 4.3 branch.

Discussion
----------

[Messenger] Disable the SchemaAssetsFilter when setup the transport

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #31623
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Commits
-------

8cbb8f8 [Messenger] Disable the SchemaAssetsFilter when setup the transport
@fabpot fabpot mentioned this pull request May 28, 2019
fabpot added a commit that referenced this pull request Apr 8, 2024
…es on setup (MatTheCat)

This PR was merged into the 5.4 branch.

Discussion
----------

[Messenger] Make Doctrine connection ignore unrelated tables on setup

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #49108
| License       | MIT

Introspecting the whole database can cause issues like Doctrine crashing on unknown column types. This PR updates the schema assets filter (added by #31625) to only consider the messages table, as it is the only one a connection setup needs to care about.

Commits
-------

22dab67 [Messenger] Make Doctrine connection ignore unrelated tables on setup
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.

5 participants