Modify

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#11120 closed defect (wontfix)

patch for Trac 0.12

Reported by: Giuseppe Ursino Owned by: obs
Priority: normal Component: RenameTracUsersScript
Severity: normal Keywords:
Cc: Steffen Hoffmann 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 Giuseppe Ursino 4 years ago.
patch.diff (597 bytes) - added by Giuseppe Ursino 4 years ago.

Download all attachments as: .zip

Change History (9)

Changed 4 years ago by Giuseppe Ursino

Attachment: main.py added

comment:1 Changed 4 years ago by Ryan J Ollos

Cc: Steffen Hoffmann 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 4 years ago by Giuseppe Ursino

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 4 years ago by Giuseppe Ursino

Attachment: patch.diff added

comment:3 Changed 4 years ago by Ryan J Ollos

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 4 years ago by Steffen Hoffmann

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 4 years ago by Giuseppe Ursino

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

comment:6 Changed 3 years ago by Ryan J Ollos

Resolution: wontfix
Status: newclosed

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 3 years ago by Steffen Hoffmann

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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain obs.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.