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

Comply with Django async syntax #22

Merged
merged 27 commits into from
Jul 8, 2023
Merged

Conversation

Archmonger
Copy link
Contributor

@Archmonger Archmonger commented May 6, 2023

Changelog

  • fix comply with official django async orm syntax #21
    • Try to use Django's own a* functions (if they exist)
    • Create a* query functions
  • Add deprecation warning to async_* query functions
  • Fix dependency hell
  • Test Django 4+
    • Add DEFAULT_AUTO_FIELD to test app
    • Bump minimum Python version to 3.8 (was easier then creating a new GitHub CI for testing to avoid Py3.7+Dj4)

@Archmonger
Copy link
Contributor Author

Archmonger commented May 6, 2023

Tests seem to be broken on Windows

PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'testdb.sqlite3'

So I'll rely on the GitHub workflow to test this PR.

@rednaks
Copy link
Owner

rednaks commented May 9, 2023

Thank you @Archmonger , the idea was to keep both the new django syntax and keep the old one, just to avoid breaking things for those who still use the current syntax.

But perhaps we can add a Deprecation warning for the old one.

@Archmonger
Copy link
Contributor Author

Sure will do

@Archmonger
Copy link
Contributor Author

Archmonger commented May 9, 2023

Btw do you want to avoid replacing existing Django methods?

For example, if the user's Django version already has acreate defined, should we avoid monkey patching it? Would help for future compatibility when Django adds real async support.

@Archmonger Archmonger marked this pull request as ready for review May 10, 2023 00:27
@Archmonger
Copy link
Contributor Author

I highly recommend doing a squash merge on this one, since the commit log is quite messy.

@rednaks
Copy link
Owner

rednaks commented May 11, 2023

Thank you @Archmonger for this amazing work, I will look at it and squash-merge if everything is good !

@Archmonger
Copy link
Contributor Author

Archmonger commented May 15, 2023

Let me know when you can review this PR.

After this gets merged, I'll PR the following

@Archmonger
Copy link
Contributor Author

@rednaks Can you either merge this PR or add me as a maintainer?

I'd like to get his repo into a production-ready state.

@rednaks rednaks merged commit 453d74d into rednaks:main Jul 8, 2023
@Archmonger Archmonger deleted the new_syntax branch August 25, 2023 07:55
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.

comply with official django async orm syntax
2 participants