Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#2440 closed defect (fixed)

Remove warnings from functions deprecated in 2.1.0

Reported by: realityexists Owned by: robe
Priority: medium Milestone: PostGIS 2.1.1
Component: postgis Version: 2.1.x
Keywords: Cc:

Description

As per the comments in #1994, please remove the warnings from functions deprecated in 2.1.0, at least in cases where the new function was only introduced in 2.1.0.

I don't think it's safe for me to update to the new functions yet, because not all my users may have upgraded to 2.1.0. Meanwhile, those who have upgraded get logs full of spurious warnings.

Change History (14)

comment:1 by robe, 11 years ago

Owner: changed from pramsey to robe

comment:2 by robe, 11 years ago

For a 2.1.1 fix I'm planning to keep the function in but just have it not do anything. Basically quiet it. That will allow for people to get back the old beahviro wiht just a new binary.

Then for 2.2.0 since we have come GUC happy either make it a GUC with default being warn and option that people can turn it off or just have it go back to same behavior of warning.

comment:3 by robe, 11 years ago

Resolution: fixed
Status: newclosed

Relegating from WARNING to DEBUG notice.

Done for 2.1 at r11964 and for 2.2 at r11962 and r11963

comment:4 by robe, 11 years ago

oops forgot to actually change function in last commit. Changed for 2.1 at r11965

comment:5 by strk, 11 years ago

Resolution: fixed
Status: closedreopened

I think this change shouldn't been made. The old signature is gone, you shouldn't use it. If you tried using a deprecated signature in raster you'd get an ERROR. A WARNING is a courtesy we do to the user. If it annoys you, change your code to use the correct signature.

comment:6 by strk, 11 years ago

After reading the comment in #1994, what I can suggest is that the _postgis_deprecate function checks the version of the deprecation (it gets it as an argument) and if it's at distance 1 it raises a NOTICE, if it is +1 raises a WARNING.

But DEBUG is really way too low IMHO.

comment:7 by strk, 11 years ago

I've a working version of my version-dependent idea. Will commit shortly.

comment:8 by strk, 11 years ago

See r11987 -- I've kept DEBUG for the deprecating version and raised to WARNING for any subsequent version. But I still think it should be NOTICE at the very least. Keeping open for comments.

comment:9 by strk, 11 years ago

Resolution: fixed
Status: reopenedclosed

r11989 in trunk (which gets you back the WARNING, given you had all of 2.1.x to fix your client code...)

comment:10 by strk, 11 years ago

Sorry, it's also r11988 in trunk, I messed up with commit logs (r11989 fixes the testsuite)

comment:11 by realityexists, 11 years ago

The old signature is not "gone" - it still works just fine. And it probably wouldn't take much effort to keep it working for at least a couple of major releases.

The problem with NOTICE is that it's typically on by default, so the message would still flood the logs. But the problem with DEBUG is that it's off by default. :)

Ideally, this would be something the user could control. So by default it might be NOTICE or even WARNING, so that users become aware of deprecated functions. But then the user could say "OK, I know about this now, I'll fix it when I can, but for now I want to turn off this message". Could it be a PostgreSQL setting (eg. "SET postgis_deprecation_log_level = DEBUG") or something similar?

comment:12 by realityexists, 11 years ago

If this the above is too hard then I'd suggest a 3-stage approach:

Release N adds a DEBUG message, ie. "you should start thinking about fixing it, but you still have time" Release N+1 change it to a WARNING message, ie. "you really should fix this now" Release N+2 removes the function

This way, even if nobody saw the DEBUG message and developers didn't become aware of the deprecation until release N+1, they can still change the code straight-away as long as all the users are no more than 1 major version behind (release N).

comment:13 by strk, 11 years ago

The patch I committed implements the 3-stage approach already.

comment:14 by realityexists, 11 years ago

Yes, of course - my bad. :) I think that should work well.

Note: See TracTickets for help on using tickets.