[issue17272] request.full_url: unexpected results on assignment

Roundup Robot added the comment: New changeset e6d862886e5c by R David Murray in branch 'default': whatsnew: urllib Request objects are now reusable. http://hg.python.org/cpython/rev/e6d862886e5c -- ___ Python tracker

[issue17272] request.full_url: unexpected results on assignment

Senthil Kumaran added the comment: This is fixed in 3.4. I dont think backporting is a good idea just to support assignment to .full_url. Thinking of this further, the idea of reusing request by assigning to .full_url may not be a good idea, because if you set .full_url to a completely differe

[issue17272] request.full_url: unexpected results on assignment

Roundup Robot added the comment: New changeset 51c5870144e7 by Senthil Kumaran in branch 'default': Fix #17272 - Make Request.full_url and Request.get_full_url return same result under all circumstances. http://hg.python.org/cpython/rev/51c5870144e7 -- _

[issue17272] request.full_url: unexpected results on assignment

Senthil Kumaran added the comment: Here is patch with tests and docs. I see no changes to opener is required and the selector which is sent to HTTP request is the correct one. I have added tests for redirect url with #fragment too (For testing scenario reported in Issue 8280). I shall close t

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: Issue 8280 reports the error by way of urlopen(). In light of that, would it not make more sense to have the *Opener be responsible for determining which parts of the url should be used? i.e. request.py, line ~1255: r.url = req.get_full_url() might instead be

[issue17272] request.full_url: unexpected results on assignment

Senthil Kumaran added the comment: I have committed the first patch which makes Request.full_url a descriptor. As I was looking at the changes to be introduced by second patch, I noticed that we do not have comprehensive test coverage for .full_url public attribute. All the tests are testing t

[issue17272] request.full_url: unexpected results on assignment

Roundup Robot added the comment: New changeset 2d4189e9bbe8 by Senthil Kumaran in branch 'default': Issue #17272: Making the urllib.request's Request.full_url a descriptor. Fixes http://hg.python.org/cpython/rev/2d4189e9bbe8 -- nosy: +python-dev ___

[issue17272] request.full_url: unexpected results on assignment

Changes by Demian Brecht : Added file: http://bugs.python.org/file29995/request_17272.2.full_url_w_fragment.patch ___ Python tracker ___ ___

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: As suggested by Senthil, I've broken this up into two patches: One that implements this reusable Request (this one) and one that implements the new (consistent) behaviour, in having full_url return the fragment. I will also add a bug/patch requesting the deprec

[issue17272] request.full_url: unexpected results on assignment

R. David Murray added the comment: Senthil, are you saying (in the review) that we should treat the current return value of full_url as a bug, and fix it in maintenance releases? It is certainly true that its value does not match the documentation. Ah. I see that get_full_url used to have th

[issue17272] request.full_url: unexpected results on assignment

R. David Murray added the comment: Thanks for working on this, Demian. I made some review comments, mostly style things about the tests. There's one substantial comment about the change in behaivor of the full_url property though (before patch it does not include the fragment, after the patch

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: Patch updated (acks update, fixed order) per Terry's instructions on core-mentorship. -- Added file: http://bugs.python.org/file29589/request_17272.1.patch ___ Python tracker _

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: Based on your and Andrew's comment in issue #16464 (new behaviour so it should only apply to 3.4), I agree that this should be a 3.4-only change. -- ___ Python tracker

[issue17272] request.full_url: unexpected results on assignment

R. David Murray added the comment: Oh, data being a property is 3.4 only. So if consistency is the goal, this should definately be a 3.4 only change. -- ___ Python tracker ___

[issue17272] request.full_url: unexpected results on assignment

R. David Murray added the comment: Having looked at the current handling of the data attribute in the context of another issue, I am now inclined to agree with you that full_url should be updated in order to have the API have a consistent behavior. Although it isn't backward incompatible API

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: No worries. The change is not backwards incompatible. test_urllib2 test pass without any modification (I'm getting a segfault in test_heapq atm so can't run the full suite). I've simply moved the side effects cause by __init__ to a setter so that full_url may

[issue17272] request.full_url: unexpected results on assignment

Senthil Kumaran added the comment: Sorry for taking long time to respond. full_url has been in the shape it is for many versions, changing it in backwards incompatible way be do more harm than good. - I would be really interested to know why the present form of full_url presented any limitati

[issue17272] request.full_url: unexpected results on assignment

Changes by Ezio Melotti : -- nosy: +ezio.melotti ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.py

[issue17272] request.full_url: unexpected results on assignment

Terry J. Reedy added the comment: I believe the issue of reusing request objects after modification has come up before, either on the tracker (but I cannot find an issue) or elsewhere. Senthil may remember better and certainly may have an opinion. I agree that python-idea would be a better pla

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: Yes, I realized that I had forgotten to add the "do nothing" option after posting but figured it was relatively obvious :) "Python doesn't normally make things read-only just because mutating them does nothing useful. Sometimes we make things read-only when mu

[issue17272] request.full_url: unexpected results on assignment

R. David Murray added the comment: One of those other options is: do nothing :) Python doesn't normally make things read-only just because mutating them does nothing useful. Sometimes we make things read-only when mutating them does nasty stuff, but even then sometimes we don't. So the real

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: It could entirely be my lack of being Dutch that didn't make this obvious without reading the docs ;) I guess there's one of three ways that this could go: To help clarify the intention through code, either a) Change full_url to _full_url, indicating that this

[issue17272] request.full_url: unexpected results on assignment

R. David Murray added the comment: Well, full_url was not designed to be assignable, and is documented that way (the docs say it is the URL passed to the constructor). Whether or not it is reasonable to make it assignable is an interesting question. -- nosy: +r.david.murray type: ->

[issue17272] request.full_url: unexpected results on assignment

Demian Brecht added the comment: I also meant to mention that of course, the obvious workaround would simply be to instantiate a new Request object with the new URL, but in my mind, this is something that should likely be supported by the Request object itself. --

[issue17272] request.full_url: unexpected results on assignment

New submission from Demian Brecht: When assigning a value to an already instantiated Request object's full_url, unexpected results are found as a consequence in the attributes selector, type and fragment. Selector, type and fragment are only assigned to during instantiation. Unless you know to