Ticket #10627 (assigned enhancement)

Opened 6 months ago

Last modified 6 months ago

Subscriber for milestones

Reported by: rjollos Assigned to: rjollos (accepted)
Priority: normal Component: AnnouncerPlugin
Severity: normal Keywords: milestone notification
Cc: hasienda Trac Release:

Description

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

Attachments

t10627-r12407-1.patch (7.4 kB) - added by rjollos on 12/06/12 11:43:13.
Patch against r12407 of the trunk.

Change History

11/15/12 21:23:13 changed by rjollos

  • status changed from new to assigned.

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.

(follow-up: ↓ 4 ) 11/15/12 21:43:23 changed by rjollos

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.

(follow-up: ↓ 5 ) 11/16/12 13:29:08 changed by rjollos

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.

(in reply to: ↑ 2 ; follow-up: ↓ 6 ) 11/17/12 21:15:06 changed by hasienda

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?

(in reply to: ↑ 3 ) 11/17/12 21:35:49 changed by hasienda

  • keywords set to milestone notification.

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.

(in reply to: ↑ 4 ) 11/19/12 05:11:59 changed by rjollos

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.

12/06/12 11:43:13 changed by rjollos

  • attachment t10627-r12407-1.patch added.

Patch against r12407 of the trunk.

12/06/12 11:45:30 changed by rjollos

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.


Add/Change #10627 (Subscriber for milestones)




Change Properties
Action