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

[Cleanup] Don't use django fork in build script #485

Open
c24t opened this issue Sep 10, 2020 · 2 comments
Open

[Cleanup] Don't use django fork in build script #485

c24t opened this issue Sep 10, 2020 · 2 comments
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. cleanup priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.

Comments

@c24t
Copy link
Contributor

c24t commented Sep 10, 2020

CI tests use a fork of django: https://github.com/c24t/django/tree/spanner/stable/2.2.x. This fork includes many changes required to run the django tests with spanner. See the full list here.

Decide how to handle this fork long term, and ensure that it's up to date with the most-recently released version of django (for each version we support). Check that we still need these patches after fast-forwarding the underlying django branch in #598.

Was previously:

The CI build script is using @timgraham's fork of django:

mkdir -p $DJANGO_TESTS_DIR && git clone --depth 1 --single-branch --branch spanner-2.2.x https://github.com/timgraham/django.git $DJANGO_TESTS_DIR/django

It looks like this was added in 27222a3, but I can't tell why we needed to use this instead of django/django. Maybe because @timgraham merged a fix on his fork before it it was available on main?

In any case we should remove this now if we can. Related to the test cleanup work in #471.

@c24t c24t added cleanup type: cleanup An internal cleanup or hygiene concern. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Sep 10, 2020
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner-django API. label Sep 10, 2020
@c24t c24t added this to the Beta milestone Oct 22, 2020
@FirePing32
Copy link

@c24t Can I take this issue ?

@c24t
Copy link
Contributor Author

c24t commented Dec 1, 2020

@prakhargurunani if you're willing to give it a shot, definitely! @IlyaFaer may have more detail on the reasons we can't just use mainline django now.

@c24t c24t removed this from the 11/5 Preview Release milestone Feb 4, 2021
@vi3k6i5 vi3k6i5 changed the title Don't use django fork in build script [Cleanup] Don't use django fork in build script Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. cleanup priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
2 participants