Ticket #1330 (closed defect: fixed)

Opened 4 years ago

Last modified 3 years ago

Undeprecate url.URL.fromRequest now that the context is going away

Reported by: dialtone Assigned to: oubiwann
Priority: highest Milestone:
Component: Nevow Severity: critical
Keywords: Cc: oubiwann
Estimated Completion (YYYY/MM/DD): Branch:
Author:

Description

The following patch removes the deprecation warning from url.URL.fromRequest. This is especially useful now that Element and the future Page object are passed only a Request and not the Context. 

Index: nevow/url.py
===================================================================
--- nevow/url.py        (revision 7775)
+++ nevow/url.py        (working copy)
@@ -94,11 +94,6 @@
     fromString = classmethod(fromString)
 
     def fromRequest(klass, request):
-        import warnings
-        warnings.warn(
-            "[v0.4] URL.fromRequest will change behaviour soon. Use fromContext instead",
-            DeprecationWarning,
-            stacklevel=2)
         uri = request.prePathURL()
         if '?' in request.uri:
             uri += '?' + request.uri.split('?')[-1]

Change History

07/16/06 10:53:48 changed by exarkun

  • owner changed from exarkun to dialtone.

Shall we also deprecate fromContext at the same time?

07/16/06 19:42:44 changed by dialtone

Probably yes but not before #1252 is merged because currently it's a legitimate usage.

07/17/06 14:13:38 changed by glyph

  • keywords deleted.

Dropping "review" tag, since it doesn't seem to make sense in this configuration of assignee and reporter.

07/17/06 14:50:31 changed by mg

But, but, but ... fromRequest() and fromContext() are two quite different things in my opinion.

fromContext() returns the URL up to the segments that have been handled by the locateChild() process so far.

fromRequest() is the wrong name for what it was doing, hence the deprecation message about a future "change [in] behaviour". fromRequest() should return the full URL.

Perhaps fromRequest() should be removed now and fromContext() should be renamed to something more descriptive?

07/17/06 19:34:18 changed by dialtone

Then I think they should both change name and be both deprecated.

Since the context is going away and url.URL.fromContext returns the current one we should call it: url.URL.currentChild(request) or similar.

url.URL.fromRequest might be a good name for the full url indeed and it does make sense.

08/03/06 08:18:48 changed by dialtone

I'm going to implement this in ticket #1252 as explained in my last ticket while not yet deprecating fromContext but adding currentChild and finally changing fromRequest.

08/03/06 08:56:56 changed by dialtone

http://divmod.org/svn/Divmod/branches/no-more-context-1252-4

This is the url for the branch that also implements a fix for this issue.

Finally I used the two following methods:

url.URL.fromHere() as a replacement for the current fromContext behavior. url.URL.fromRequest() now returns the url.URL for the full request.uri url.URL.fromContext() not deprecated and works as it does right now.

08/03/06 09:02:03 changed by dialtone

  • keywords set to review.

11/04/06 17:00:22 changed by jml

  • owner changed from dialtone to exarkun.

If it's up for review, it has to be assigned to someone else.

I'd review it, but I'm really not qualified -- I don't know what the plan is wrt removing context.

exarkun, feel free to explain and pass back to me for review.

11/04/06 18:54:06 changed by exarkun

  • owner changed from exarkun to jml.

This should be taken care of but I have no time, even to explain it. I was planning to take care of this post-ship. The process doesn't really have any provisions for high-cost, non-critical, review tasks, unfortunately.

11/05/06 06:30:44 changed by glyph

  • owner deleted.

Taking care of this with Twisted's style of review.

02/28/07 16:38:07 changed by oubiwann

So we've had some conversations on IRC about this, but I'm not super clear on this and do want to clarify:

  • are we planning to include this patch and (temporarily... or not) eliminate data and slots?
  • or are we going to add a new flatten package, providing this optional but faster API?

03/01/07 11:42:11 changed by amir

  • owner set to exarkun.

Since this is highest priority it needs an owner and/or a change or priority. Assigning to JP to delegate.

03/05/07 08:20:27 changed by exarkun

  • keywords deleted.
  • owner changed from exarkun to oubiwann.
  • We absolutely should stop pointing at fromContext as a useful thing
  • We should probably undeprecate fromRequest as well
  • fromContext should probably be deprecated - assuming it is exactly identical to fromRequest (which is how it appears to me) - and until it is removed, should be implemented in terms of fromRequest
  • A new method should be added which allows an URL which represents the current traversal state to be constructed (but this probably bears a separate ticket)

03/05/07 08:54:02 changed by exarkun

Note dialtone had a comment above which I didn't read until after making my previous comment which answers most of the same questions as my previous comment.

Our answers aren't exactly the same, but they're similar. ;)

03/25/07 21:26:50 changed by oubiwann

  • cc set to oubiwann.
  • status changed from new to assigned.

07/03/07 10:29:57 changed by exarkun

  • status changed from assigned to closed.
  • resolution set to fixed.

Well, somehow fromRequest got deprecated. There's a good amount of discussion about other things which should be done in this ticket, but since it's other things, there's no reason to keep this ticket open.

jethro@divmod.org