Modify

Opened 18 months ago

Closed 11 months ago

Last modified 11 months ago

#11120 closed defect (wontfix)

patch for Trac 0.12

Reported by: giursino Owned by: obs
Priority: normal Component: RenameTracUsersScript
Severity: normal Keywords:
Cc: hasienda Trac Release: 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.

Attachments (2)

main.py (5.3 KB) - added by giursino 18 months ago.
patch.diff (597 bytes) - added by giursino 18 months ago.

Download all attachments as: .zip

Change History (9)

Changed 18 months ago by giursino

comment:1 Changed 18 months ago by rjollos

  • Cc hasienda added; anonymous removed

Hi, Would you kindly attach you changes as a patch file? See t:TracDev/SubmittingPatches for details if you aren't familiar with the procedure.

I believe this plugin will soon be deprecated since it will soon be released as part of the AccountManagerPlugin. However, we might want to apply the patch for the interm.

comment:2 Changed 18 months ago by giursino

I've attached patch file. I think that this plugin is very speedy and comfortable to use it without install AccountManagerPlugin. Please consider to maintain it either as standalone plugin.

Changed 18 months ago by giursino

comment:3 follow-up: Changed 18 months ago by rjollos

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.

Can you post a traceback of the error you were seeing?

comment:4 in reply to: ↑ 3 Changed 18 months ago by hasienda

Replying to rjollos:

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].

comment:5 Changed 18 months ago by giursino

Ok. I accept your suggestion. My db is PostgreSQL, so the bug is known.

comment:6 follow-up: Changed 11 months ago by rjollos

  • Resolution set to wontfix
  • Status changed from new to closed

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.

comment:7 in reply to: ↑ 6 Changed 11 months ago by hasienda

Replying to rjollos:

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

Modify Ticket

Action
as closed The owner will remain obs.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.