Skip to content

Conversation

@solarissmoke
Copy link
Collaborator

@solarissmoke solarissmoke commented Feb 21, 2021

This will hopefully flag changes to the Treebeard models that
require a migration, to ensure that such changes are (a) evaluated for
their necessity and (b) shipped as part of a major release.

See #212 .

@tabo
Copy link
Member

tabo commented Feb 21, 2021

Excellent! I’m on my phone right now but I’ll take a closer look when I’m back on the PC. I see mssql tests failed? That seems odd

@solarissmoke
Copy link
Collaborator Author

solarissmoke commented Feb 21, 2021

MySQL failures seem unrelated - maybe just need to restart those builds (I don't seem to have permission to do that)?

@tabo
Copy link
Member

tabo commented Feb 25, 2021

It just failed the tests for MSSQL again. Don't know much about them, but it's the second time this PR makes them fail. I don't know much about SQL Server, and even less about Django connecting to it, but if we are supporting it now, we need to sort this out.

@solarissmoke
Copy link
Collaborator Author

MSSQL failures are because django-mssql-backend isn't compatible with Django>3.0 - it's been fixed on master but they've not cut a release.

@tabo
Copy link
Member

tabo commented Feb 25, 2021

Oh! Can we disable those tests until that's dealt with then?

@solarissmoke
Copy link
Collaborator Author

@tabo yes - I've commented those out of the appveyor config for now. Build should pass this time.

This will hopefully flag changes to the Treebeard models that
require a migration, to ensure that such changes are (a) evaluated for
their necessity and (b) shipped as part of a major release.
@solarissmoke
Copy link
Collaborator Author

solarissmoke commented Feb 25, 2021

A general comment on MSSQL - Django itself does not support MSSQL, and there really isn't a robust third-party backend available. I don't think Treebeard can actually guarantee that it will work on MSSQL - and on that basis don't think we should declare support for it.

@solarissmoke solarissmoke requested a review from tabo February 25, 2021 05:26
@tabo tabo merged commit 374740c into django-treebeard:master Feb 25, 2021
@tabo
Copy link
Member

tabo commented Feb 25, 2021

@solarissmoke Fantastic work on this PR. Thanks! And you make a fair point on MSSQL support, but if the tests are working with it, then it's safe to say that treebeard will work on it. My only concern is having 2 test infra and how slow those MSSQL tests are.

@solarissmoke
Copy link
Collaborator Author

@tabo it should be possible to move these tests to Github actions (thus we can drop both Travis and AppVeyor) - I will have a play with that when I have some more time.

@solarissmoke solarissmoke deleted the feature/migration-tests branch February 25, 2021 05:43
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.

2 participants