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

Hydrate AbstractSqlExecutor::sqlStatements #11026

Closed
wants to merge 1 commit into from

Conversation

greg0ire
Copy link
Member

This test caused deprecations because
AbstractSqlExecutor::_sqlStatements, a property that no longer exists since it was renamed was hydrated instead.

This test caused deprecations because
AbstractSqlExecutor::_sqlStatements, a property that no longer exists
since it was renamed was hydrated instead.
@greg0ire
Copy link
Member Author

greg0ire commented Oct 25, 2023

@derrabus when working on this yesterday, I was happy to see I managed to fix the breaking test, and then I saw other test broke, involving classes inheriting from the one I modified. I see 3 possibilites now:

  1. Removing support for unserializing 2.x format
  2. Fixing the newly broken tests, and accepting the fact that executors outside this repository might break as well if they have properties.
  3. Doing something on 2.x, maybe with __sleep / __serialize so that we store forward-compatible representations, in addition to 1.

What do you think?

@greg0ire
Copy link
Member Author

Alternate PR: #11027

@greg0ire greg0ire closed this Nov 8, 2023
@greg0ire greg0ire deleted the fix-unserialize branch November 8, 2023 21:17
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.

1 participant