Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#12285 closed defect (fixed)

[PATCH] Replace deprecated env.get_db_cnx() method

Reported by: mikhael@… Owned by: osimons
Priority: normal Component: XmlRpcPlugin
Severity: normal Keywords: get_db_cnx
Cc: Olemis Lang Trac Release: 1.2

Description

Environment.get_db_cnx() method was marked as deprecated and was removed in Trac 1.1.1.

See also: ​http://trac.edgewall.org/ticket/11605

Attachments (1)

get_db_cnx.patch (2.1 KB) - added by mikhael@… 7 years ago.

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by mikhael@…

Attachment: get_db_cnx.patch added

comment:1 Changed 7 years ago by Ryan J Ollos

osimons: is it okay if we create a 1.0 branch for supporting Trac 1.0 and later?

comment:2 Changed 7 years ago by osimons

The RPC plugin should really only need to interface against API level of Trac, not DB access. There are only 2 uses of db = self.env.get_db_cnx() that I can find, and they are from the very oldest part of the code for simple SELECT access.

For those two simple uses I would much prefer if we could make compat code that handled this gracefully and avoided branching. Either avoid db access at all by finding Trac API calls to use, or just a simple conditional check to handle alternative call mechanisms.

When the time comes that branching can't be avoided, we should branch out the old code in a "latest-supported-version" branch, and trunk should be where the future changes happens. Depending on where the line is drawn that would mean making a 0.12 branch or a 1.0 branch, and implementing changes in trunk for anything newer.

comment:3 Changed 7 years ago by Ryan J Ollos

I guess the question then is whether anyone is actually going to produce that compatibility patch, or if the plugin just ends up sitting in a state where it's not compatible with the soon-to-be-released Trac 1.2. I've spent a lot of time supporting old versions of Trac, and I just don't feel that it's a worthwhile use of my time anymore, especially considering how much work there is to be done and how few patches are contributed by the community. My approach now is to put the burden on those that want to use the latest plugin versions to upgrade their Trac instance. I suppose though if you have your own cases for supporting older versions of Trac then it may be worthwhile to you to keep the plugin backward-compatible.

comment:4 Changed 7 years ago by Olemis Lang

FWIW I agree with osimons . In principle IMHO there should be some Trac API for us to get ticket data without worrying about DB connection . I also prefer handling this gracefully rather than branching .

comment:5 Changed 7 years ago by Ryan J Ollos

The important thing is that something is committed, otherwise XmlRpcPlugin will be compatible back to 0.11, and soon not compatible with the latest version of Trac. It would be simple to branch and apply the patch. However, if we wait for some ideal solution that no one produces a patch for, XmlRpcPlugin will be useless and I'll have to remove it again when upgrading trac-hacks.org.

comment:6 Changed 7 years ago by osimons

Resolution: fixed
Status: newclosed

In 14743:

XmlRpcPlugin: Support Trac 1.1+ database API. Closes #12285.

comment:7 Changed 7 years ago by Ryan J Ollos

Thank you for pushing a fix. One step closer to trac-hacks running the forthcoming Trac 1.2!

comment:8 in reply to:  5 ; Changed 7 years ago by Olemis Lang

Replying to rjollos:

[...]

It would be simple to branch and apply the patch.

As you can see , no , it's better to maintain a single branch , especially if the changes needed are relatively simple .

Nevertheless , I'm still of the opinion that in XmlRpcPlugin we should not be executing inline SQL queries like this one . There should be something in core for the plugin to retrieve ticket / wiki data . You may say I'm a dreamer but I'm not the only one .

comment:9 in reply to:  8 ; Changed 7 years ago by osimons

Replying to olemis:

I'm still of the opinion that in XmlRpcPlugin we should not be executing inline SQL queries like this one . There should be something in core for the plugin to retrieve ticket / wiki data .

Of course we agree. But even then the SQL would be needed as compat code to support older versions of Trac... "Recent changes" for wiki and ticket is only provided through timeline APIs in Trac, but digesting HTML output from timeline providers is certainly not a better option. As you know... ;-)

comment:10 Changed 7 years ago by Ryan J Ollos

osimons: could you bump the version number for these changes? It's harder to track the installed version now that setuptools doesn't support tag_svn_revision.

comment:11 Changed 7 years ago by osimons

r14747 => v1.1.4

comment:12 in reply to:  9 Changed 7 years ago by Olemis Lang

Replying to osimons:

Replying to olemis:

I'm still of the opinion that in XmlRpcPlugin we should not be executing inline SQL queries like this one . There should be something in core for the plugin to retrieve ticket / wiki data .

Of course we agree.

good to know

But even then the SQL would be needed as compat code to support older versions of Trac...

yes , but it's not the same to have on one hand some code in a known-to-be-phased-out compat.py file executing SQL queries for old (i.e. frozen) versions of Trac and on the other hand the stable and development versions invoking methods defined in core , which might vary as Trac evolves but hopefully keeping the same (or compatible) signature .

"Recent changes" for wiki and ticket is only provided through timeline APIs in Trac,

If this is still the case it's again the same thing : inline SQL in Trac core rather than encapsulation ; because in fact ticket and wiki UI do not rely upon timeline providers for rendering neither wiki version history nor ticket comments .

but digesting HTML output from timeline providers is certainly not a better option. As you know... ;-)

What I'm certain of is that there is an RPC handler for timeline providers out there , in case users might need it . At least in theory , the difficulty involved in processing this data stream does not make inline SQL right .

comment:13 Changed 7 years ago by Olemis Lang

osimons I'm not suggesting to use timeline providers to implement ticket and wiki RPC methods , I'm just saying that it'd be nice if something could be done about inline SQL .

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain osimons.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.