Skip to content
This repository

Django 1.4 - minor compatibility issue #14

Merged
merged 3 commits into from about 2 years ago

9 participants

tomscytale Jonas Obrist Eric Palakovich Carr Adrián Ribao Andi Albrecht Tom Morris Matteo Parrucci Chris McKenzie Emilian Felecan
tomscytale

No description provided.

Jonas Obrist
Owner
ojii commented

Thanks for this pull request.

I'm not really a fan of try...except... where it isn't required, so maybe this should use getattr

tomscytale

updated - please look again.

Jonas Obrist
Owner
ojii commented

Alright. Now had a closer look. Does Django 1.4 just remove that attribute? It's not replaced with anything?

tomscytale

and...
django_trunk/django$ grep -r parent_name_expr *
django_trunk/django$

Eric Palakovich Carr

Any movement on this?

Jonas Obrist
Owner
ojii commented

Not really sure this fix will actually work as it will result in a Variable.resolve or FilterExpression.resolve being called with None. Which will break. What needs to happen is there should be a check for the extend_node being dynamic or not.

tomscytale
tomscytale
Adrián Ribao

I used this patch in a project usign django 1.4 and everything works fine.

Jonas Obrist
Owner
ojii commented

Thanks for verifying @AdrianRibao

@tomscytale in the old version, if parent_name_expr the context was set to None but never used. Now with this change in Django and your patch the None context will actually be used and I feared that this might cause issues.

tomscytale

are you sure? as far as I can tell the context is used unless extend_node.partent_name_expr is True. And usually it is False. So in normal circumstances context is None, right?

Andi Albrecht

FWIW this is the change that removed parent_name_expr: django/django@5cedcb4
And the corresponding Django ticket: https://code.djangoproject.com/ticket/17660

Jonas Obrist
Owner
ojii commented

Thanks andi, I will still need to actually test this pull request before I can merge it, this will hopefully happen next week when I should have a little more time to work on this.

Tom Morris

Is it next week yet? :-)

The patch worked for me and the current code definitely will not work with Django 1.4.

Matteo Parrucci

The patch is working here. The old code does not work...
News on testing for merge?

Adrián Ribao

This patch works, it should be merged ASAP. It breaks 1.4 I've been using it for a long time in production enviroments.

Chris McKenzie
kenzic commented

I second what @AdrianRibao, @parruc, and @tfmorris said.

Eric Palakovich Carr

Thirded.

Jonas Obrist
Owner
ojii commented

Noted, will merge this/make sekizai 1.4 compatible post-djangocon europe

Jonas Obrist ojii merged commit 5018f46 into from
Jonas Obrist ojii closed this
Jonas Obrist
Owner
ojii commented

just noticed that this patch actually breaks a test. will investigate

Jonas Obrist
Owner
ojii commented

the issue is exactly what I described above, get_parent is called with None on a dynamic extends tag.

Emilian Felecan

Is there a better patch to get this working with Django 1.4?

Jonas Obrist
Owner

I merged the patch. sekizai master now supports up to the in-development 1.5 release of Django. (On Python versions up to 3.2).

I just gotta make a proper release for the new version.

Geoffrey Hing ghing referenced this pull request in denverfoundation/storybase
Closed

Upgrade to Django 1.4 #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 2 additions and 2 deletions. Show diff stats Hide diff stats

  1. +2 2 sekizai/helpers.py
4 sekizai/helpers.py
@@ -10,7 +10,7 @@ def _extend_blocks(extend_node, blocks):
10 10 Extends the dictionary `blocks` with *new* blocks in the parent node (recursive)
11 11 """
12 12 # we don't support variable extensions
13   - if extend_node.parent_name_expr:
  13 + if hasattr(extend_node, 'parent_name_expr') and extend_node.parent_name_expr:
14 14 return
15 15 parent = extend_node.get_parent(None)
16 16 # Search for new blocks
@@ -36,7 +36,7 @@ def _extend_nodelist(extend_node):
36 36 ExtendsNode
37 37 """
38 38 # we don't support variable extensions
39   - if extend_node.parent_name_expr:
  39 + if hasattr(extend_node, 'parent_name_expr') and extend_node.parent_name_expr:
40 40 return []
41 41 blocks = extend_node.blocks
42 42 _extend_blocks(extend_node, blocks)

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.