[Django] #25400: Attributes are always set on `connection.features` (gis)
Django
----------------------------------------------+------------------------
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords: regression
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+------------------------
With commit d9ff5ef36d3f714736d633435d45f03eac9c17b5 the behaviour of
`hasattr`
on `connection.features` seems to have changed, e.g. the following will be
True
now always, and was previously so only for Django < 1.5.
{{{
hasattr(connection.features, 'confirm')
}}}
pytest-django is using the following code:
{{{
# confirm() is not needed/available in Django >= 1.5
# See https://code.djangoproject.com/ticket/17760
if hasattr(self.connection.features, 'confirm'):
self.connection.features.confirm()
}}}
The problem seems to be that `__getattr__` should raise an AttributeError
in this case.
(Source:
https://github.com/django/django/commit/d9ff5ef36d3f714736d633435d45f03eac9c17b5
#diff-fbce17c4cb97f431da78cbdf3ac4ff16R90)
--
Ticket URL: <https://code.djangoproject.com/ticket/25400>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: regression | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* severity: Normal => Release blocker
* needs_better_patch: => 0
* needs_tests: => 1
* needs_docs: => 0
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Thanks for noticing that regression. Could you please add a test to the
patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/25400#comment:1>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: regression | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by blueyed):
Where should the test go?
`tests/gis_tests/gdal_tests/test_ds.py`?
--
Ticket URL: <https://code.djangoproject.com/ticket/25400#comment:2>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: regression | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
A new file called `tests/gis_tests/test_backends.py` seems reasonable to
me.
--
Ticket URL: <https://code.djangoproject.com/ticket/25400#comment:3>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: regression | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by blueyed):
It might make sense to add a test for this for all backends?
(with pytest I could imagine using a parametrized test for this - which
would access an existing and non-existing attribute).
Can this be done in Django's testing framework?
Apart from that, I think that it would be better/faster if somebody who is
more used to Django's tests would add it.
(I am more used to tests based on pytest, and to be honest: the lack of
proper attribution (in terms of Git authorship) for (several, but not all)
previous patches from me makes me less motivated to go the last mile here,
although that seems to get handled better by now - just saying).
--
Ticket URL: <https://code.djangoproject.com/ticket/25400#comment:4>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: regression | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I think writing the test for the current backend being tested (e.g.
checking `hasattr(connection.features, 'nonexistent')` is enough.
--
Ticket URL: <https://code.djangoproject.com/ticket/25400#comment:5>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: regression | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"da5747f8e460ea2a2d16f124463b3975ccfc0992" da5747f8]:
{{{
#!CommitTicketReference repository=""
revision="da5747f8e460ea2a2d16f124463b3975ccfc0992"
Fixed #25400 -- Fixed regression in nonexistent features on gis backends.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25400#comment:6>