#11915 closed defect (fixed)
Environment.get_db_cnx is deprecated
Reported by: | Owned by: | Ryan J Ollos | |
---|---|---|---|
Priority: | highest | Component: | AccountManagerPlugin |
Severity: | blocker | Keywords: | get_db_cnx |
Cc: | Jun Omae, Franz | Trac Release: | 1.2 |
Description
Due to the new support for automated transaction management, Connection instances should no longer be acquired using the familiar get_db_cnx method.
http://trac.edgewall.org/wiki/TracDev/ApiChanges/0.12#get_db_cnx
Attachments (3)
Change History (43)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Priority: | highest → high |
---|---|
Trac Release: | 0.12 → 1.2 |
Thanks to Ryan for answering before me.
To clarify furthermore, I'm not interested in adopting 0.12 db API as well, because it would mean an intermediate step mainly for keeping Python2.4 compatibility just a little longer. Looking into sources you may recognize, that I already pushed that to extremes.
The inevitable fork will radical too: Just Trac 1.0 API and consequent drop of Python2.4 compatibility. And we must follow Trac core soon now to not loose application fitness, right.
comment:3 follow-up: 4 Changed 10 years ago by
I don't understand the plan (i.e. it is not clear *if* a fork will be made or not), but in the meantime the attached patch works for me, using AMP with current Trac trunk. By any chance it is not good enough to be integrated as is, because I cannot even test some of the ramifications...
comment:4 follow-up: 5 Changed 10 years ago by
Replying to lgaifax:
I don't understand the plan (i.e. it is not clear *if* a fork will be made or not),
Obviously branching is mandatory to adapt Trac 1.0 db API. I just hesitate to develop in 0.11
and 1.0
branches for this plugin in parallel.
but in the meantime the attached patch works for me, using AMP with current Trac trunk. By any chance it is not good enough to be integrated as is, because I cannot even test some of the ramifications...
Thanks for pushing the issue. While moving to the new API I'll review and try to improve the code. Breaking portions of your patch is not intended, but inevitable in that process. Until forking trunk
to 1.0
branch I plan to offer updated patches for Trac 1.0 API here. Hopefully this will satisfy early adopters using Trac 1.1.2 or later development versions.
comment:5 follow-ups: 6 9 Changed 10 years ago by
Cc: | Jun Omae added; anonymous removed |
---|
Replying to hasienda:
Replying to lgaifax:
I don't understand the plan (i.e. it is not clear *if* a fork will be made or not),
Obviously branching is mandatory to adapt Trac 1.0 db API. I just hesitate to develop in
0.11
and1.0
branches for this plugin in parallel.
I think We could use with_transaction
decorator in trac.db.api
which still is available in Trac 1.1.2 instead of Environment.get_db_cnx
(dropped in 1.1.2) and Environment.db_transaction
(introduced in 1.0 and only works on Python 2.5+).
See https://github.com/jun66j5/accountmanagerplugin/compare/t11915. In the branch, get_read_db
function and with_transaction
decorator are added to acct_mgr.compat
. Thoes functions work well on Trac 0.11 through 1.1.2.
BTW, I found another incompatibility with Trac 1.1.2. OrderedExtensionsOption
raises ConfigurationError
for unavailable components since 1.1.2.
====================================================================== ERROR: test_render_cfg_admin_panel (acct_mgr.tests.admin.AccountManagerAdminPanelTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jun66j5/src/accountmanagerplugin.git/acct_mgr/tests/admin.py", line 162, in test_render_cfg_admin_panel 'config', '') File "/home/jun66j5/src/accountmanagerplugin.git/acct_mgr/admin.py", line 227, in render_admin_panel return self._do_config(req) File "/home/jun66j5/src/accountmanagerplugin.git/acct_mgr/admin.py", line 258, in _do_config list=self.acctmgr.register_checks) File "/home/jun66j5/venv/trac/1.1.2dev-r13069/lib/python2.6/site-packages/trac/config.py", line 795, in __get__ option=tag.code("[%s] %s" % (self.section, self.name)))) ConfigurationError: Cannot find implementation(s) of the <code>IAccountRegistrationInspector</code> interface named <code>DisabledCheck</code>. Please check that the Component is enabled or update the option <code>[account-manager] register_check</code> in trac.ini.
comment:6 follow-up: 7 Changed 10 years ago by
Replying to jun66j5:
Replying to hasienda:
Replying to lgaifax:
I don't understand the plan (i.e. it is not clear *if* a fork will be made or not),
Obviously branching is mandatory to adapt Trac 1.0 db API. I just hesitate to develop in
0.11
and1.0
branches for this plugin in parallel.I think We could use
with_transaction
decorator intrac.db.api
which still is available in Trac 1.1.2 instead ofEnvironment.get_db_cnx
(dropped in 1.1.2) andEnvironment.db_transaction
(introduced in 1.0 and only works on Python 2.5+). See https://github.com/jun66j5/accountmanagerplugin/compare/t11915. In the branch,get_read_db
function andwith_transaction
decorator are added toacct_mgr.compat
. Thoes functions work well on Trac 0.11 through 1.1.2.
I see. But this is what I refer to as Trac 0.12 db API. Sorry, but I don't feel that its worth touching the db code twice. In fact I'm already half through with creating the changeset for the conversion to "with env.db_..".
BTW, I found another incompatibility with Trac 1.1.2.
OrderedExtensionsOption
raisesConfigurationError
for unavailable components since 1.1.2.
Uh. I'm unaware of related changes and will have to check that first.
comment:7 follow-up: 8 Changed 10 years ago by
Replying to hasienda:
BTW, I found another incompatibility with Trac 1.1.2.
OrderedExtensionsOption
raisesConfigurationError
for unavailable components since 1.1.2.Uh. I'm unaware of related changes and will have to check that first.
To help with refreshing memory, the change was implemented in trac:#10285.
comment:8 Changed 10 years ago by
Replying to rjollos:
To help with refreshing memory, the change was implemented in trac:#10285.
Thanks. Now I remember indeed. Then it does not only affect mentioned development versions leading to Trac 1.2 but current stable Trac 1.0.2 or later too.
comment:9 Changed 10 years ago by
Replying to jun66j5:
See https://github.com/jun66j5/accountmanagerplugin/compare/t11915. In the branch,
get_read_db
function andwith_transaction
decorator are added toacct_mgr.compat
. Thoes functions work well on Trac 0.11 through 1.1.2.
There is a bunch of additional pointers on what to look for, including unit tests. Thanks a lot, even if I don't intend to keep Python2.4 compatibility any longer for the upcoming development branch.
Changed 10 years ago by
Attachment: | web_ui.py.diff added |
---|
Patch for acct_mgr/web_ui.py
to work with Trac 1.1.3
comment:11 Changed 10 years ago by
Priority: | high → highest |
---|
Newest Account-Manager from Trunk (SVN 14395) is not working with Trac version 1.1.3. For fixing the issue web_ui.py.diff could be applied.
Error log shows:
ERROR: Internal Server Error: Traceback (most recent call last): File "build\bdist.win32\egg\trac\web\main.py", line 551, in _dispatch_request dispatcher.dispatch(req) File "build\bdist.win32\egg\trac\web\main.py", line 186, in dispatch chosen_handler = self._get_valid_default_handler(req) File "build\bdist.win32\egg\trac\web\main.py", line 279, in _get_valid_default_handler name = req.session.get('default_handler') File "build\bdist.win32\egg\trac\web\api.py", line 384, in __getattr__ value = self.callbacks[name](self) File "build\bdist.win32\egg\trac\web\main.py", line 306, in _get_session return Session(self.env, req) File "build\bdist.win32\egg\trac\web\session.py", line 202, in __init__ if req.authname == 'anonymous': File "build\bdist.win32\egg\trac\web\api.py", line 384, in __getattr__ value = self.callbacks[name](self) File "build\bdist.win32\egg\trac\web\main.py", line 140, in authenticate authname = authenticator.authenticate(req) File "build\bdist.win-amd64\egg\acct_mgr\util.py", line 81, in wrap return func(self, *args, **kwds) File "build\bdist.win-amd64\egg\acct_mgr\web_ui.py", line 425, in authenticate return auth.LoginModule.authenticate(self, req) File "build\bdist.win32\egg\trac\web\auth.py", line 91, in authenticate req.incookie['trac_auth']) File "build\bdist.win-amd64\egg\acct_mgr\web_ui.py", line 496, in _get_name_for_cookie db = self.env.get_db_cnx() AttributeError: 'Environment' object has no attribute 'get_db_cnx'
comment:12 Changed 10 years ago by
Cc: | Franz added |
---|
comment:13 Changed 10 years ago by
I'm planning to upgrade trac-hacks.org to Trac 1.1.3 or 1.1.4 (#12171), hopefully in the next month or so. This ticket is a critical step to accomplishing that.
In the past I've tried to be strict in maintaining plugins and support versions of Trac back to 0.11. Recently I've been more inclined to just cut a final release supporting Trac < 1.0, even if it's not perfect, and move on to supporting Trac >= 1.0. I simply don't have the time to do everything I want to do, so I have to make some compromises somewhere. My thought is, what's better for the Trac project, supporting a user on 0.11.x, or supporting early adopters of Trac 1.1.x? If you are in the same boat as me time-wise, I think it would be good to just cut the 0.5 release and move on to supporting Trac >= 1.0. It will allow more to be done for AccountManagerPlugin in whatever little time we have available, and may even have the nice side effect of forcing users to upgrade to 1.0+.
comment:14 follow-up: 15 Changed 9 years ago by
Ticket #12506 was closed as duplicate.
Fortunately acct_mgr-trac1.1.patch of lgaifax is working fine. Thanks for that.
As lgaifax, I do not understand why AccountManager is kept backwards compatible; aren't branches made for such cases? Another possibily would be to make a version 0.6 for >= 1.1.2 and keep an older version 0.5 for older Trac versions < 1.1.
comment:15 Changed 9 years ago by
Replying to framay:
Another possibily would be to make a version 0.6 for >= 1.1.2 and keep an older version 0.5 for older Trac versions < 1.1.
That was the original plan. I'd just like to hear from hasienda whether he will be able to continue maintaining the project, or if that is just not possible for him to do any longer. I can't maintain the project long-term, but I'm willing to create the 0.5 and 0.6 releases.
It's often the case developers find they no longer have time to maintain these projects and we shouldn't fault anyone for that. Some brief communication about the status though would be a big help to those that depend on the project so that we can find a way to forge ahead.
comment:18 Changed 9 years ago by
Indeed it seems the obvious thing to do would be to create a branch for older trac versions, and move trunk to supporting the new database API.
rjollos, if it's alright with you and/or hasienda I'd be willing to help with maintenance of this project at least insofar as it getting it minimally working on current Trac (1.1.6). Otherwise I'll probably have to fork it and that's no fun :(
comment:19 follow-up: 20 Changed 9 years ago by
The trunk code supports Trac >= 1.0, which is a change from the previous plan (comment:3:ticket:12722). I'm not bothering with any versions of Trac prior to 1.0. If someone makes a patch that applies cleanly against the trunk I can review and apply it.
comment:20 Changed 9 years ago by
Replying to rjollos:
The trunk code supports Trac >= 1.0, which is a change from the previous plan (comment:3:ticket:12722). I'm not bothering with any versions of Trac prior to 1.0. If someone makes a patch that applies cleanly against the trunk I can review and apply it.
Okay, thanks for the update. I'll submit a patch sometime next week. Cheers!
Changed 9 years ago by
Attachment: | acct_mgr_db_api.patch added |
---|
Attached patch upgrades trunk to use the Trac 1.0 DB API only. Not sure about much else, but I did confirm with this patch that all the unit tests pass. I couldn't get the functional tests to work due to mysterious problems with Twill.
comment:21 Changed 9 years ago by
Owner: | changed from Steffen Hoffmann to Ryan J Ollos |
---|---|
Status: | new → accepted |
The functional tests haven't been working for me as well. The patch looks good. I'll test and apply it this week. Thanks!
comment:23 Changed 8 years ago by
Tests are passing on OSX after changes in #12192. I need to refactor acct_mgr_db_api.patch so that it conforms to Trac idioms, but should have changes committed within a few days.
comment:25 Changed 8 years ago by
As an example,
with self.env.db_transaction as db: cursor = db.cursor() cursor.execute("INSERT INTO session_attribute " "(sid,authenticated,name,value) " "VALUES (%s,1,'password',%s)", ('bar', 'bar'))
can become:
self.env.db_transaction("INSERT INTO session_attribute " "(sid,authenticated,name,value) " "VALUES (%s,1,'password',%s)", ('bar', 'bar'))
There are other cases where it's not necessary to obtain a cursor.
Also, in some cases we can use:
for item, in self.env.db_query(""" ...
rather than:
with self.env.db_query as db: for item, in db(""" ....
This is documented at trac:TracDev/DatabaseApi#Trac1.0API.
comment:26 Changed 8 years ago by
Ah, I didn't know about the iterable interface for db_query
. Nice!
comment:27 Changed 8 years ago by
If you have some time to do the refactoring and rebase the patch on the latest trunk, that would be much appreciated. Otherwise, I will try to get it committed soon, but pretty busy with finalizing Trac 1.2!
All unit test pass for me on OSX. I'd be interested to hear if they pass on other platforms.
comment:28 Changed 8 years ago by
I've been working on these changes and plan to have them committed tomorrow. I hope followers of this ticket will be willing to install the latest and give feedback. Thanks!
comment:29 Changed 8 years ago by
Are there any updated patches that one can apply on top of trunk to get this going? My current installation of trac got updated to 1.2 and it is now crippled because it's just unusable without the account manager plugin. I am now debating if I should go through the pain of a downgrade or... I don't know what else? Suggestions?
comment:30 Changed 8 years ago by
See comment:28.
Upgrading your site without creating a backup or running the upgrade first on a staging site is a very bad idea. But if that is where you are now, you can probably get attachment:acct_mgr_db_api.patch to apply with little or no edits. That, or you could downgrade and wait. I may not finish this today, but will have it finished by end of week.
comment:31 Changed 8 years ago by
I have all the backups I need, that's not the problem, churn is. The trac 1.2 update came in as a system update and I would rather not pin outdated package versions if I can avoid it, even if it means doing a little bit of extra work.
As for the patch, unless there is a new one, this does not apply against trunk, I've tried (fixing patch context by hand where needed) and said in comment:29 : Are there any updated patches that one can apply. I'll wait a few days and hope an updated patch or commit is posted. Thanks
comment:33 Changed 8 years ago by
Yes, it will be done soon. It's in the critical path for a number of my projects, however there is a lot of testing to do, and other AccountManager issues to resolve to make it compatible with Trac 1.2.
comment:35 Changed 8 years ago by
I've read all comments but I do not understand why trunk does not contain needed changes(in the end active version of trac is 1.2). I've build the source from trunk but it does obviously not run with trac 1.2.
I've also tried patch from comment 30 with no success. Would you clear out at what revision should be applied patch from comment 30?
comment:37 Changed 8 years ago by
Just curious - what branch is this patch on? It doesn't appear to be in the trunk yet?
comment:39 follow-up: 40 Changed 7 years ago by
Can't you just attach the egg that will work with the new version?
How do I apply these patches?
comment:40 Changed 7 years ago by
Replying to Christiaan@…:
How do I apply these patches?
Install from trunk of Subversion repository. See TracPlugins#Fromsource. Please ask on the MailingList if you that's not clear.
We are aware of this, but hasienda has chosen to keep the plugin backward compatible, which is a good choice. However,
get_db_cnx
has been dropped in Trac 1.1.2, so the changes must be made eventually: trac:wiki:TracDev/ApiChanges/1.1.2#DatabaseAPIChanges.