Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#13086 closed defect (wontfix)

Hashed options are not portable

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Component: PollMacro
Severity: normal Keywords:
Cc: Trac Release:

Description

After upgrading trac-hacks.org to Trac 1.2 most of the poll results are not displayed. The following pages have polls:

Only RequestaHack shows any votes.

The poll options are hashed: pollmacro/trunk/tracpoll/tracpoll.py@16266:166#L146.

For the poll on the PollMacro page, the hashed values in the .poll file are:

  • Cool -> 13fc52bf
  • Crap -> 73addc7c

On the trac-hacks.org server, I get:

>>> s1 = 'Cool'
>>> '%08x' % abs(hash(s1))
'16f6c6ee13fc52bf'
>>> s2 = 'Crap'
>>> '%08x' % abs(hash(s2))
'16f6da078c522384'

I've gotten closer to reproducing the pair of values using the first 32bits of the hash, but only the first string matches:

>>> '%08x' % abs(hash(s1) & ((1<<32)-1))
'13fc52bf'
>>> '%08x' % abs(hash(s2) & ((1<<32)-1))
'8c522384'

I assume the polls displayed correctly before upgrading to Trac 1.2, which involved creating a new virtualenv, but I get the same results from the old virtualenv so it's unclear to me what has changed.

I think we need to move to database storage of polls and results (comment:4:ticket:3495), but it would be nice to recover the results first.

Attachments (1)

t13086.diff (905 bytes) - added by Ryan J Ollos 7 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by Jun Omae

It seems old virtualenv is based on 32-bit python.

>>> import sys
>>> sys.maxsize
2147483647
>>> import platform
>>> platform.architecture()
('32bit', 'ELF')
>>> '%x' % abs(hash('Cool'))
'13fc52bf'
>>> '%x' % abs(hash('Crap'))
'73addc7c'

Use low 32-bit as signed integer to match stored hashed values in .poll file:

>>> import sys
>>> sys.maxsize
9223372036854775807
>>> import platform
>>> platform.architecture()
('64bit', 'ELF')
>>> '%08x' % abs(struct.unpack('>i', struct.pack('>I', hash('Crap') & 0xffffffff))[0])
'73addc7c'
Last edited 7 years ago by Jun Omae (previous) (diff)

comment:2 Changed 7 years ago by Jun Omae

Another method is to use ctypes.c_int:

>>> '%08x' % abs(ctypes.c_int(hash('Crap')).value)
'73addc7c'

Changed 7 years ago by Ryan J Ollos

Attachment: t13086.diff added

comment:3 Changed 7 years ago by Ryan J Ollos

Resolution: wontfix
Status: newclosed

Thanks! I patched with attachment:t13086.diff. Now we can move storage to the database (#3495).

comment:4 Changed 7 years ago by Jun Omae

BTW, ticket notification is not received when I added comment:1 and comment:2 to this ticket. Before upgrading to Trac 1.2, notifications was sent by [notification] always_notify_updater enabled.

It seems TicketUpdaterSubscriber and TicketPreviousUpdatersSubscriber are missing in [notification-subscriber]. The 2 subscribers are automatically added by source:/tags/trac-1.2/trac/upgrades/db40.py@:33-38#L17.

comment:5 Changed 7 years ago by Ryan J Ollos

Thanks for spotting. .db40.bak has:

[notification]
always_notify_owner = true
always_notify_reporter = true

The default for always_notify_updater is True, so the TicketUpdaterSubscriber and TicketPreviousUpdatersSubscriber should be present, but there is only:

[notification-subscriber]
always_notify_cc = CarbonCopySubscriber
always_notify_owner = TicketOwnerSubscriber
always_notify_reporter = TicketReporterSubscriber

The TicketUpdaterSubscriber and TicketPreviousUpdatersSubscriber values are also missing from .db41.bak, so it seems the upgrade step must have failed to add them.

I added them manually, so should be fixed now:

[notification-subscriber]
always_notify_cc = CarbonCopySubscriber
always_notify_owner = TicketOwnerSubscriber
always_notify_reporter = TicketReporterSubscriber
always_notify_updater = TicketUpdaterSubscriber
always_notify_previous_updater = TicketPreviousUpdatersSubscriber

comment:6 Changed 7 years ago by Jun Omae

Thanks. I just received the notification now.

comment:7 in reply to:  5 ; Changed 7 years ago by Jun Omae

Replying to Ryan J Ollos:

The default for always_notify_updater is True, so the TicketUpdaterSubscriber and TicketPreviousUpdatersSubscriber should be present, but there is only:

This is a defect in db40.py. We should confirm default value of [notification] always_notify_* options.

comment:8 in reply to:  7 Changed 7 years ago by Ryan J Ollos

Replying to Jun Omae:

This is a defect in db40.py. We should confirm default value of [notification] always_notify_* options.

Thanks, I thought the issue sounded familiar. Looks to follow the same pattern as the issue that was fixed in [trac 14152].

comment:9 in reply to:  7 Changed 7 years ago by Ryan J Ollos

Replying to Jun Omae:

Replying to Ryan J Ollos:

The default for always_notify_updater is True, so the TicketUpdaterSubscriber and TicketPreviousUpdatersSubscriber should be present, but there is only:

This is a defect in db40.py. We should confirm default value of [notification] always_notify_* options.

Issue reported in trac:#12700.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
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.