Re: Moving database backends out of the core
Hi Shai, Thank you. You make a very good point. That is exactly what I meant. I have written small backends myself to add new features. The problem is that those features rarely make it into the core backend, because those are too static inside Django. Best -Joe On 28. Nov 2018, 00:33 +0100, Shai Berger , wrote: > Hi, > > On Mon, 26 Nov 2018 00:57:04 -0800 (PST) > Johannes Hoppe wrote: > > > I want to address a completely different point, and that > > *innovation*. I believe that 3rd party backends could lead to more > > innovation in the Django ORM. > > Currently if you want to introduce a new feature for your database, > > you are faced with a lot of complications added by databases you > > might not be familiar with. Furthermore you might be requested to > > makes those features available for databases you haven't used before. > > This drastically increases the bar for contributing innovative new > > features. As an example, I wanted to add database defaults for > > Postgres and multiple insert return values. I finished the postgres > > feature in 2 sprints, but it took me another half year to implement > > the same for Oracle. Mainly because I never used Oracle before. > > > > I just wanted to point out that this road to innovation is already > open. There's nothing stopping you from writing a custom database > backend and releasing it to PyPI. Granted, then your new feature does > not become part of Django, but that would also be the case if the > standard backends were not part of Django. > > I should point out that this point is not theoretical -- e.g. an > extended Oracle backend existed to support connection pooling[1], and > when I wanted to add database instrumentation[2], I first implemented > it (though I hadn't published that) in a custom backend, inheriting the > existing PG backend. > > Shai > > [1] https://github.com/JohnPapps/django-oracle-drcp > [2] https://docs.djangoproject.com/en/2.1/topics/db/instrumentation/ > > -- > You received this message because you are subscribed to a topic in the Google > Groups "Django developers (Contributions to Django itself)" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/django-developers/O-g06EM6XMM/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > django-developers+unsubscr...@googlegroups.com. > To post to this group, send email to django-developers@googlegroups.com. > Visit this group at https://groups.google.com/group/django-developers. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/20181128013339.2901e7a7.shai%40platonix.com. > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/b28aa44f-bf40-4d2b-bffc-d323f1aa05d9%40Spark. For more options, visit https://groups.google.com/d/optout.
Re: TestCase.setUpTestData in-memory data isolation.
> > Are these the kind of objects you assign during setUpTestData though? > They're not the kind of things *I* assign but I bet somewhere out there, some project does extensively :) What about going through a deprecation period where non-picklable > assignment during setUpTestData raises a deprecation warning suggesting > using setUpClass or direct class attributes assignment? > I like this idea, it's actually likely to be easy to implement too (on top of the descriptor). On Tue, 27 Nov 2018 at 08:59, charettes wrote: > Thanks for chiming in Adam. > > I think this is too much of an ask for backwards compatibility. Lots of >> things aren't deepcopy-able, as per its docs: >> > > Right. Are these the kind of objects you assign during setUpTestData > though? They could be easily moved to setUpTestCase for example. > > How about adding a container object to TestCase, which deepcopy()'s its >> attributes on setUp. It could be called something short like "data', which >> would make your example: >> > > That seems like a viable alternative. > > My only concerns are that it requires adjusting all existing usages of > setUpTestData to assign to cls.data instead. It looks like it could be easy > to forget doing so until you actually hit a data isolation issue. That > makes setUpTestData unsafe to use unless you assign to data. That also > creates two data "world" where objects assigned to cls and cls.data lose > their references so altering self.data.book.author wouldn't affect > self.author if the latter is not assigned to data. > > What about going through a deprecation period where non-picklable > assignment during setUpTestData raises a deprecation warning suggesting > using setUpClass or direct class attributes assignment? Another alternative > could be to silently ignore deepcopy failures and return the original > objects in these cases. > > Simon > > Le dimanche 25 novembre 2018 03:53:48 UTC-5, Adam Johnson a écrit : >> >> I have run into this problem myself in the past. On a previous project we >> added a helper function to make deepcopy's of named attributes during >> setUp(). >> >> From a check against a few projects and Django's test suite[2] I have only >>> identified a single issue which is that attributes assigned during >>> `setUpTestData` would now have to be `deepcopy()`able but it shouldn't be >>> a blocker given `Model` instance are. >> >> >> I think this is too much of an ask for backwards compatibility. Lots of >> things aren't deepcopy-able, as per its docs: >> >> This module does not copy types like module, method, stack trace, stack >>> frame, file, socket, window, array, or any similar types. It does “copy” >>> functions and classes (shallow and deeply), by returning the original >>> object unchanged; this is compatible with the way these are treated by the >>> pickle module. >> >> >> How about adding a container object to TestCase, which deepcopy()'s its >> attributes on setUp. It could be called something short like "data', which >> would make your example: >> >> class BookTests(TestCase): >> @classmethod >> def setUpTestData(cls): >> cls.data.author = Author.objects.create() >> cls.data.book = cls.author.books.create() >> >> def test_relationship_preserved(self): >> self.assertIs(self.data.book.author, self.data.author) >> >> On Sat, 24 Nov 2018 at 03:29, charettes wrote: >> >>> Dear developers, >>> >>> Django 1.8 introduced the `TestCase.setUpTestData()` class method as a >>> mean to >>> speed up test fixtures initialization as compared to using `setUp()`[0]. >>> >>> As I've come to use this feature and review changes from peers using it >>> in >>> different projects the fact that test data assigned during its execution >>> couldn't be safely altered by test methods without compromising test >>> isolation >>> has often be the source of confusion and frustration. >>> >>> While the `setUpTestData` documentation mentions this limitation[1] and >>> ways to >>> work around it by using `refresh_from_db()` in `setUp()` I believe it >>> defeats >>> the whole purpose of the feature; avoiding unnecessary roundtrips to the >>> database to speed up execution. Given `TestCase` goes through great >>> lengths to >>> ensure database level data isolation I believe it should do the same >>> with class >>> level in-memory data assigned during `setUpTestData`. >>> >>> In order to get rid of this caveat of the feature I'd like to propose an >>> adjustment to ensure such in-memory test data isolation. >>> >>> What I suggest doing is wrapping all attributes assigned during >>> `setUpTestData` >>> in descriptors that lazily return `copy.deepcopy()`ed values on instance >>> attribute accesses. By attaching the `deepcopy()`'s memo on test >>> instances we >>> can ensure that the reference graph between objects is preserved and thus >>> backward compatible. >>> >>> In other words, the following test would pass even if `self.book` is a >>> deep >>> copy of `cls.book`. >>> >
Re: TestCase.setUpTestData in-memory data isolation.
Our project also suffers extensively with mutating objects assigned from setUp, preventing us from moving most of our tests to setUpTestData. I'll likely begin using your pypi package right away, thanks Simon! Backward compat issues are probably likely - but they'd be in test cases exclusively, making them extremely easy to find during an upgrade. That said, a deprecation warning is probably the most sensible path forward to prevent the need for immediate action. Is there anyway to determine the pickle-ability of something without just trying to pickle it? I wouldn't be keen on that overhead. Could you just capture any copy exceptions, raise a deprecation warning, and abandon the copy for that attribute? On Saturday, 24 November 2018 14:29:33 UTC+11, charettes wrote: > > Dear developers, > > Django 1.8 introduced the `TestCase.setUpTestData()` class method as a > mean to > speed up test fixtures initialization as compared to using `setUp()`[0]. > > As I've come to use this feature and review changes from peers using it in > different projects the fact that test data assigned during its execution > couldn't be safely altered by test methods without compromising test > isolation > has often be the source of confusion and frustration. > > While the `setUpTestData` documentation mentions this limitation[1] and > ways to > work around it by using `refresh_from_db()` in `setUp()` I believe it > defeats > the whole purpose of the feature; avoiding unnecessary roundtrips to the > database to speed up execution. Given `TestCase` goes through great > lengths to > ensure database level data isolation I believe it should do the same with > class > level in-memory data assigned during `setUpTestData`. > > In order to get rid of this caveat of the feature I'd like to propose an > adjustment to ensure such in-memory test data isolation. > > What I suggest doing is wrapping all attributes assigned during > `setUpTestData` > in descriptors that lazily return `copy.deepcopy()`ed values on instance > attribute accesses. By attaching the `deepcopy()`'s memo on test instances > we > can ensure that the reference graph between objects is preserved and thus > backward compatible. > > In other words, the following test would pass even if `self.book` is a deep > copy of `cls.book`. > > class BookTests(TestCase): > @classmethod > def setUpTestData(cls): > cls.author = Author.objects.create() > cls.book = cls.author.books.create() > > def test_relationship_preserved(self): > self.assertIs(self.book.author, self.author) > > Lazily returning `deepcopy'ies and caching returned values in `__dict__` à > la > `cached_property` should also make sure the slight performance overhead > this > incurs is minimized. > > From a check against a few projects and Django's test suite[2] I have only > identified a single issue which is that attributes assigned during > `setUpTestData` would now have to be `deepcopy()`able but it shouldn't be > a blocker given `Model` instance are. > > In order to allow other possible issues from being identified against > existing > projects I packaged the proposed feature[3] and made it available on > pypi[4]. It > requires decorating `setUpTestData` methods but it shouldn't be too hard to > apply to your projects if you want to give it a try. > > Given this reaches consensus that this could be a great addition I'd file > a ticket and finalize what I have so far[2]. > > Thank your for your time, > Simon > > [0] > https://docs.djangoproject.com/en/1.8/releases/1.8/#testcase-data-setup > [1] > https://docs.djangoproject.com/en/2.1/topics/testing/tools/#django.test.TestCase.setUpTestData > [2] > https://github.com/charettes/django/compare/setuptestdata...charettes:testdata > [3] https://github.com/charettes/django-testdata > [4] https://pypi.org/project/django-testdata/ > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/df0991aa-18d3-4fcd-999d-7226a64faf75%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: TestCase.setUpTestData in-memory data isolation.
Hey Josh, glad the package can help in the mean time! > Is there anyway to determine the pickle-ability of something without just trying to pickle it? I wouldn't be keen on that overhead. Not that I'm aware off but unfortunately. There really shouldn't be much overhead though because the deepcopy is only performed on instance attribute access. That means that tests that are only creating test data without accessing attributes assigned during setUpTestData() are not going to be affected by this change at all. In other words, I suggest only doing the deep copy if required for the requested attributes so if you define a complex data set in setUpTestData() then only the few attributes and their related objects would get deepcopied on instance attribute accesses. > Could you just capture any copy exceptions, raise a deprecation warning, and abandon the copy for that attribute? Yeah that's the exact plan for the deprecation phase; warn and return the actual attribute. From Django 3.1 this would result in an AttributeError. I've updated the branch with the deprecation warnings[0] approach. I'll give it one or two more weeks to gather a bit more feedback but it looks like this could be a viable option so far. Cheers, Simon [0] https://github.com/django/django/compare/master...charettes:testdata#diff-5d7d8ead1a907fe91ffc121f830f2a49R1032 Le mercredi 28 novembre 2018 21:40:53 UTC-5, Josh Smeaton a écrit : > > Our project also suffers extensively with mutating objects assigned from > setUp, preventing us from moving most of our tests to setUpTestData. I'll > likely begin using your pypi package right away, thanks Simon! > > Backward compat issues are probably likely - but they'd be in test cases > exclusively, making them extremely easy to find during an upgrade. That > said, a deprecation warning is probably the most sensible path forward to > prevent the need for immediate action. > > Is there anyway to determine the pickle-ability of something without just > trying to pickle it? I wouldn't be keen on that overhead. Could you just > capture any copy exceptions, raise a deprecation warning, and abandon the > copy for that attribute? > > On Saturday, 24 November 2018 14:29:33 UTC+11, charettes wrote: >> >> Dear developers, >> >> Django 1.8 introduced the `TestCase.setUpTestData()` class method as a >> mean to >> speed up test fixtures initialization as compared to using `setUp()`[0]. >> >> As I've come to use this feature and review changes from peers using it in >> different projects the fact that test data assigned during its execution >> couldn't be safely altered by test methods without compromising test >> isolation >> has often be the source of confusion and frustration. >> >> While the `setUpTestData` documentation mentions this limitation[1] and >> ways to >> work around it by using `refresh_from_db()` in `setUp()` I believe it >> defeats >> the whole purpose of the feature; avoiding unnecessary roundtrips to the >> database to speed up execution. Given `TestCase` goes through great >> lengths to >> ensure database level data isolation I believe it should do the same with >> class >> level in-memory data assigned during `setUpTestData`. >> >> In order to get rid of this caveat of the feature I'd like to propose an >> adjustment to ensure such in-memory test data isolation. >> >> What I suggest doing is wrapping all attributes assigned during >> `setUpTestData` >> in descriptors that lazily return `copy.deepcopy()`ed values on instance >> attribute accesses. By attaching the `deepcopy()`'s memo on test >> instances we >> can ensure that the reference graph between objects is preserved and thus >> backward compatible. >> >> In other words, the following test would pass even if `self.book` is a >> deep >> copy of `cls.book`. >> >> class BookTests(TestCase): >> @classmethod >> def setUpTestData(cls): >> cls.author = Author.objects.create() >> cls.book = cls.author.books.create() >> >> def test_relationship_preserved(self): >> self.assertIs(self.book.author, self.author) >> >> Lazily returning `deepcopy'ies and caching returned values in `__dict__` >> à la >> `cached_property` should also make sure the slight performance overhead >> this >> incurs is minimized. >> >> From a check against a few projects and Django's test suite[2] I have only >> identified a single issue which is that attributes assigned during >> `setUpTestData` would now have to be `deepcopy()`able but it shouldn't be >> a blocker given `Model` instance are. >> >> In order to allow other possible issues from being identified against >> existing >> projects I packaged the proposed feature[3] and made it available on >> pypi[4]. It >> requires decorating `setUpTestData` methods but it shouldn't be too hard >> to >> apply to your projects if you want to give it a try. >> >> Given this reaches consensus that this could be a great addition I'd file >> a ticket and finalize w
