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.

Sign up for GitHub

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

Jump to bottom

validate schema command: allow to debug missing schema updates list #9019

Merged
merged 1 commit into from
Mar 3, 2022
Merged

validate schema command: allow to debug missing schema updates list #9019

merged 1 commit into from
Mar 3, 2022

Conversation

COil
Copy link
Contributor

@COil COil commented Sep 16, 2021

Hi,

This PR allows seeing why you get the message "The database schema is not in sync with the current mapping file" by displaying all the missing SQL so the database schema can be up-to-date. This debug is only activated when at least the verbose mode of the console is activated.

See you. COil :)

Questions:

  • Do I need to cache the update list, so there is only one call to getUpdateSchemaList?
  • Is it OK the tests the getVerbosity level? Or should it be displayed in every case (like for the mapping)?
  • Need tests? There is currently no test for the ValidateSchemaCommand if I am right (except that is callable with orm:validate-schema)

@COil COil force-pushed the ValidateSchemaCommand/show-missing-update-debug branch from 181af44 to 0e6acb8 Compare September 17, 2021 17:27
@derrabus
Copy link
Member

Thank you for your pull request. I wonder, isn't the SQL that would be dumped here be the same as the SchemaTool\UpdateCommand would dump?

@COil
Copy link
Contributor Author

COil commented Sep 17, 2021 •
edited
Loading

Thank you for your pull request. I wonder, isn't the SQL that would be dumped here be the same as the SchemaTool\UpdateCommand would dump?

Hello Derrabus, yes of course, but you have to run a second command (yes, I am lazy 😁). I liked the idea of displaying it directly. Moreover, for the mapping, the errors are displayed.

@derrabus
Copy link
Member

I see. Nevertheless, we would need tests that cover your change. Also, since this is a new feature, the 2.10.x branch would be the appropriate target.

@COil
Copy link
Contributor Author

COil commented Sep 19, 2021

Hello, I can redo the PR to be on the 2.10.x. branch. Can you give some hint about the tests that should be done? Thanks.

@greg0ire greg0ire changed the base branch from 2.9.x to 2.10.x September 19, 2021 10:45
@greg0ire greg0ire force-pushed the ValidateSchemaCommand/show-missing-update-debug branch from 0e6acb8 to 6d0f9a8 Compare September 19, 2021 10:47
@greg0ire
Copy link
Member

I can redo the PR to be on the 2.10.x. branch

No need! I re-targeted and rebased!

@COil COil force-pushed the ValidateSchemaCommand/show-missing-update-debug branch 3 times, most recently from f677206 to fcf746d Compare September 21, 2021 20:11
@COil COil force-pushed the ValidateSchemaCommand/show-missing-update-debug branch 3 times, most recently from 7e72c33 to a9c7f58 Compare September 21, 2021 20:25
@COil COil force-pushed the ValidateSchemaCommand/show-missing-update-debug branch 2 times, most recently from 75aa9d1 to 0117a4d Compare October 9, 2021 07:48
@greg0ire greg0ire changed the base branch from 2.10.x to 2.11.x October 9, 2021 08:58
@greg0ire
Copy link
Member

greg0ire commented Oct 9, 2021

The tests fail with Postgresql, Mysql, MariaDB. Why?

@COil
Copy link
Contributor Author

COil commented Oct 9, 2021

The tests fail with Postgresql, Mysql, MariaDB. Why?

The schema is in synch for Postgresql, Mysql, MariaDB so there is no error message. I guess I must force schema to be invalid.

@COil COil force-pushed the ValidateSchemaCommand/show-missing-update-debug branch from 0117a4d to 2689a52 Compare October 9, 2021 11:57
@COil
Copy link
Contributor Author

COil commented Oct 9, 2021

I skipped the test for the other platforms, as we only want to test the error output, it's not related to the db platform. Maybe it can be cleaner? let me know.

greg0ire
greg0ire previously approved these changes Oct 9, 2021
View reviewed changes
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

The platform is not intervening at any point in the new code you introduced, so IMO it's good as is.

@greg0ire greg0ire requested a review from derrabus October 9, 2021 12:31
@COil
Copy link
Contributor Author

COil commented Mar 3, 2022

Hi @derrabus, do you think there is still work to do on this PR? 🙂

@derrabus derrabus changed the base branch from 2.11.x to 2.12.x March 3, 2022 18:23
@derrabus derrabus dismissed greg0ire’s stale review March 3, 2022 18:23

The base branch was changed.

@derrabus derrabus force-pushed the ValidateSchemaCommand/show-missing-update-debug branch from 2689a52 to c434c20 Compare March 3, 2022 18:25
@derrabus
Copy link
Member

derrabus commented Mar 3, 2022

Looks good to me, I guess. Let's push it through the CI again.

@derrabus derrabus enabled auto-merge (squash) March 3, 2022 18:27
@derrabus derrabus added this to the 2.12.0 milestone Mar 3, 2022
@derrabus derrabus merged commit 89d0a6a into doctrine:2.12.x Mar 3, 2022
@derrabus
Copy link
Member

derrabus commented Mar 3, 2022

Thank you!

@COil COil deleted the ValidateSchemaCommand/show-missing-update-debug branch March 6, 2022 22:23
@COil COil restored the ValidateSchemaCommand/show-missing-update-debug branch March 6, 2022 22:23
n-e-m-a-nj-a pushed a commit to n-e-m-a-nj-a/orm that referenced this pull request Mar 29, 2022
@COil @n-e-m-a-nj-a
validate schema command: allow to debug missing schema updates list (d…
7b99585 
…octrine#9019)
@github-actions github-actions bot mentioned this pull request Apr 19, 2022
Merged
@simPod simPod mentioned this pull request Apr 26, 2022
Merged
@COil COil deleted the ValidateSchemaCommand/show-missing-update-debug branch July 7, 2022 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

@greg0ire greg0ire greg0ire left review comments

Assignees
No one assigned
Labels
Improvement
Projects
None yet
Milestone
2.12.0
Development

Successfully merging this pull request may close these issues.
3 participants
@COil @derrabus @greg0ire