patch for Trac 0.12

Description

Hi, thanks for your plugin.
I've found that it was not working on trac 0.12, because the db schema was slightly changed.
So I've attached you a patch that correct it.
Please consider to update the package.

The two entries you commented out are for supporting the VotePlugin and TracHoursPlugin. It doesn't have anything to do with the schema having changed in 0.12.

The plugin should skip over unknown tables because the check for table existence is wrapped in a try / except: cur.execute("SELECT * FROM %s LIMIT 1" % table). It could be that the method of checking for table existence is flawed. hasienda has found that method of checking for table existence fails when used in IEnvironmentSetupParticipant.environment_needs_upgrade in some dev revisions of Trac leading up to 1.0, if I understand correctly.

The two entries you commented out are for supporting the VotePlugin and TracHoursPlugin. It doesn't have anything to do with the schema having changed in 0.12.

Right. Therefor this ticket falsely claims a defect for this Trac hack, and you're patch should NOT be accepted. As Ryan told you before, you'll want to elaborate about graceful skipping of not installed plugins rather than commenting them out. This would be a true regression for other users, you see?

The plugin should skip over unknown tables because the check for table existence is wrapped in a try / except: cur.execute("SELECT * FROM %s LIMIT 1" % table). It could be that the method of checking for table existence is flawed. hasienda has found that method of checking for table existence fails when used in IEnvironmentSetupParticipant.environment_needs_upgrade in some dev revisions of Trac leading up to 1.0, if I understand correctly.

You will have no luck with PostgreSQL, don't know about MySQL, but SQLite is rather forgiving. Anyway, the correct approach would be to use db backend-specific queries for all known tables, not raising exceptions as a regular test method, see i.e. _get_tables in [12124].

Closing ticket and suggest that users direct themselves to rename support in the AccountManagerPlugin. While I expect to hear more arguments about "not having to install an entire plugin, and better to maintain it as a standalone", the fact is that it is difficult for us to maintain all these plugins, so the slight inconvenience to the user of a few extra MB download will not outweight what is convenient for developers. You can enable only the Components that are needed in AccountManagerPlugin, and it really presents no harm or burden to the end user other than getting over false concerns of installing some "big plugin" that isn't needed.

Closing ticket and suggest that users direct themselves to rename support in the AccountManagerPlugin. While I expect to hear more arguments about "not having to install an entire plugin, and better to maintain it as a standalone", the fact is that it is difficult for us to maintain ...

Add some more rational thoughts.

Without a solid base the whole rename/user ID change action would end in an undefined status. Think of committing thousands of replacements before hitting an issue mid-way through. You rely on a sane rollback, don't you?

Next is giving feedback about the exact position (db table), that might be critical for a fast fix. The current web-UI provided by the account manager admin panel is rather verbose and convenient IMHO.

And the fact remains, that AcctMgr is THE component for managing users. You'll likely need something else from the package, sooner or later, if not yet.

Add Comment

This ticket has been modified since you started editing. You should review the
other modifications which have been appended above,
and any conflicts shown in the preview below.
You can nevertheless proceed and submit your changes if you wish so.