🐛 Fix SQLAlchemy version 1.4.36 breaks SQLModel relationships (#315)

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉

[byrman]

No description provided.

[jossefaz]

Can we add unit tests ?

[byrman]

Can we add unit tests ?

What would be a good test? I don't know how to write tests for different SA versions.

[antont]

I guess just a test for relationships would work.

[byrman]

I guess just a test for relationships would work.

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

[antont]

OK, but there are plenty of those already. It would be nice if we could prove that it works for both 1.4.36 and older versions of SQLAlchemy.

Right - apparently tests just have not been run in Github Actions in 16d so the breakage does not show.

I don't think a test in itself can define the lib version but that would require something more, maybe like a branch where a different SA version is pinned.

[mkarbo]

Running tests without the fix results in broken tests, running with fix all tests pass. You can run them locally to verify

[lonelyteapot]

Any updates on when this will be merged? The issue is critical

[bharathanr]

Hi! Is there anything we can do to help with merging this pull request? It's rather critical, so if there's something that can be done, it would be great. I'm willing to put in some time to help.

[berendt]

@tiangolo Can you take a look at this, please? It is nasty that you have to pin the version of sqlalchemy at the moment. It would be nice if you could make a release after the merge so that the current version of sqlalchemy can be used again.

[leynier]

In this PR (https://github.com/leynier/sqlmodel/pull/37) pointing to a fork, you can see that the tests are successful. I think that It is not necessary to add extra tests for this PR because the existing tests validate the change since without the change of this PR the tests fail.

[ghost]

#322

#315
fix

[codecov]

Codecov Report

Merging #322 (a55ae6d) into main (4d20051) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #322 +/- ##
=======================================
Coverage 97.49% 97.49% =======================================
Files 181 181 Lines 6037 6038 +1 =======================================
+ Hits 5886 5887 +1 
Misses 151 151 

Impacted Files	Coverage Δ
sqlmodel/main.py	81.79% <100.00%> (+0.05%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit a55ae6d at: https://630a5d484e7d9624a7f35029--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @byrman! 🙇 🚀

And thanks everyone for the discussion.

This will be available in the next version, in the next hours. 🎉