Modify

Opened 11 years ago

Last modified 6 years ago

#10627 new enhancement

Subscriber for milestones

Reported by: Ryan J Ollos Owned by:
Priority: normal Component: AnnouncerPlugin
Severity: normal Keywords: milestone notification
Cc: Steffen Hoffmann Trac Release:

Description

Add subscribers for milestones created, modified, closed and deleted.

Attachments (1)

t10627-r12407-1.patch (7.4 KB) - added by Ryan J Ollos 11 years ago.
Patch against r12407 of the trunk.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 11 years ago by Ryan J Ollos

Status: newassigned

Requesting feedback on the file structure. Does the following seem correct?:

  • New subscriber goes in announcer/subscribers.
  • New subscriber is named milestone.py.
  • Tests for new subscriber go in subscribers/tests/milestone.py.

comment:2 Changed 11 years ago by Ryan J Ollos

We immediately run into an issue that I've seen before when trying to run the tests:

Traceback (most recent call last):
  File "setup.py", line 88, in <module>
    **extra
  File "/usr/lib/python2.6/distutils/core.py", line 152, in setup
    dist.run_commands()
  File "/usr/lib/python2.6/distutils/dist.py", line 975, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python2.6/distutils/dist.py", line 995, in run_command
    cmd_obj.run()
  File "/home/user/Workspace/th9110/lib/python2.6/site-packages/distribute-0.6.24-py2.6.egg/setuptools/command/test.py", line 137, in run
    self.with_project_on_sys_path(self.run_tests)
  File "/home/user/Workspace/th9110/lib/python2.6/site-packages/distribute-0.6.24-py2.6.egg/setuptools/command/test.py", line 117, in with_project_on_sys_path
    func()
  File "/home/user/Workspace/th9110/lib/python2.6/site-packages/distribute-0.6.24-py2.6.egg/setuptools/command/test.py", line 146, in run_tests
    testLoader = loader_class()
  File "/usr/lib/python2.6/unittest.py", line 816, in __init__
    self.parseArgs(argv)
  File "/usr/lib/python2.6/unittest.py", line 843, in parseArgs
    self.createTests()
  File "/usr/lib/python2.6/unittest.py", line 849, in createTests
    self.module)
  File "/usr/lib/python2.6/unittest.py", line 613, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib/python2.6/unittest.py", line 584, in loadTestsFromName
    parent, obj = obj, getattr(obj, part)
AttributeError: 'module' object has no attribute 'tests'

I've found it to be problematic to have a module and directory with the same name. One solution would be to move subscribers.py to subscribers/ticket.py.

comment:3 Changed 11 years ago by Ryan J Ollos

Capturing discussion from IRC which is a reply to comment:1 and makes comment:2 irrelevant:

  • New subscribers will go in announcer/opt/subscribers.py since they are not part of existing standard Trac notification system, with tests in announcer/opt/tests/subscribers.py.
  • A new test module will need to be created for announcer/producers.py.

The method of testing subscribers and producers by decoupling them from the distributor is still under discussion, but I'll capture that discussion here when we reach some conclusions. It sounds like a MockDistributor might be the way to go.

comment:4 in reply to:  2 ; Changed 11 years ago by Steffen Hoffmann

Replying to rjollos:

We immediately run into an issue that I've seen before when trying to run the tests: ...

This is from your patched code, right?

I've found it to be problematic to have a module and directory with the same name. One solution would be to move subscribers.py to subscribers/ticket.py.

Sure, seen this before, but I don't see, where it applies to Announcer, because we don't have such duplication yet, do we?

comment:5 in reply to:  3 Changed 11 years ago by Steffen Hoffmann

Keywords: milestone notification added

Replying to rjollos:

The method of testing subscribers and producers by decoupling them from the distributor is still under discussion, but I'll capture that discussion here when we reach some conclusions. It sounds like a MockDistributor might be the way to go.

This, or capturing all events by a catch-all filter (IAnnouncementSubscriptionFilter implementation), so wouldn't have to care for neither distributor nor transport.

But the more I look at the code, the more I think that api.SubscriptionResolver is the primary tool to test subscribers anyway.

comment:6 in reply to:  4 Changed 11 years ago by Ryan J Ollos

Replying to hasienda:

Sure, seen this before, but I don't see, where it applies to Announcer, because we don't have such duplication yet, do we?

Right, it's not an issue now that "new" subscribers are being placed in opt. comment:1 and comment:2 are no longer relevant following our discussion on IRC.

Changed 11 years ago by Ryan J Ollos

Attachment: t10627-r12407-1.patch added

Patch against r12407 of the trunk.

comment:7 Changed 11 years ago by Ryan J Ollos

The patch in t10627-r12407-1.patch is a basic implementation of Announcer support for milestones. It provides all, created, changed and deleted subscribers, and a plain text email notification. I plan to add an HTML email notification and improve the plain text template. I'm submitting the patch here for early review of how I've structured the implementation.

comment:8 Changed 11 years ago by Ryan J Ollos

Status: assignednew

comment:9 Changed 7 years ago by Ryan J Ollos

Owner: Ryan J Ollos deleted

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.

Add Comment


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

 
Note: See TracTickets for help on using tickets.