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

Prepare for Ecto 3.11 #126

Merged
merged 10 commits into from
Dec 4, 2023
Merged

Conversation

greg-rychlewski
Copy link
Contributor

@greg-rychlewski greg-rychlewski commented Sep 21, 2023

I put everything into this PR so it can be merged once you use Ecto 3.11. But you could pick out the parent_as one and commit it now (minus the new test) if you prefer

@greg-rychlewski
Copy link
Contributor Author

It looks like some new integration tests are failing. I'll take a look.

@greg-rychlewski
Copy link
Contributor Author

Ok I found the last culprit. Previously Ecto would not send nil down to the adapter but it was requested to send it through here elixir-ecto/ecto#4214.

So this means your bool_encode function would have to take nil into account.

@warmwaffles
Copy link
Member

Is there a timeline for Ecto 3.11 release?

@greg-rychlewski
Copy link
Contributor Author

Not at the moment. There is someone working on a Cassandra adapter that might ask for a release once they are ready. But other than that no real indication of when it will happen.

I don't mind keeping this up to date with the changes that happen until then.

@warmwaffles
Copy link
Member

warmwaffles commented Sep 22, 2023

I'll keep an eye out for the 3.11 release. When that lands I'll ping you and we should be able to get this squared away and released. I appreciate all the work here.

@greg-rychlewski
Copy link
Contributor Author

Ecto 3.11 was released yesterday. I updated the deps and also I added a clause to raise if someone tries to use values lists with this adapter. When I looked up how values lists are handled in sqlite it was a bit weird because you can't give column names. I'm not too sure the best way to handle it atm.

@warmwaffles
Copy link
Member

@greg-rychlewski it's been a while, it looks like 3.11 has landed. I don't remember if there is more work to be done on this.

@greg-rychlewski
Copy link
Contributor Author

Hey,

It's good to go. I added one thing since we talked last that you may want to double check. I added a clause to raise when a values list is given to your adapter. Not sure the best way to implement it atm because it seems like SQLite doesn't let you name the columns in values lists.

@warmwaffles
Copy link
Member

That's fine. SQLite does have plenty of quirky limitations 😛

@warmwaffles warmwaffles merged commit 290e96a into elixir-sqlite:main Dec 4, 2023
9 checks passed
@greg-rychlewski
Copy link
Contributor Author

@warmwaffles Btw we added docs to DB Connection that might be of interest to you. This can potentially be utilized to run async tests with SQLite. It requires opening up multiple databases but it might not be too bad elixir-ecto/db_connection@fa5f705

@greg-rychlewski greg-rychlewski deleted the ecto_311 branch December 4, 2023 13:57
@warmwaffles
Copy link
Member

Interesting, someone made a PR asking for a callback before each disconnect as well https://github.com/elixir-sqlite/exqlite/blob/5b9891bbe7d965532de6e3e8ca5c82735f864158/lib/exqlite/connection.ex#L162-L164

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.

Don't need to pattern match on {:maybe, type} in upcoming Ecto release Implement fragment splice
2 participants