Modify

Opened 2 years ago

Last modified 19 months ago

#10627 new enhancement

Subscriber for milestones

Reported by: rjollos Owned by: rjollos
Priority: normal Component: AnnouncerPlugin
Severity: normal Keywords: milestone notification
Cc: hasienda Trac Release:

Description

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

Attachments (1)

t10627-r12407-1.patch (7.4 KB) - added by rjollos 2 years ago.
Patch against r12407 of the trunk.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 2 years ago 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.

comment:2 follow-up: Changed 2 years ago 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.

comment:3 follow-up: Changed 2 years ago 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.

comment:4 in reply to: ↑ 2 ; follow-up: Changed 2 years ago 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?

comment:5 in reply to: ↑ 3 Changed 2 years ago by hasienda

  • 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 2 years ago 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.

Changed 2 years ago by rjollos

Patch against r12407 of the trunk.

comment:7 Changed 2 years ago 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.

comment:8 Changed 19 months ago by rjollos

  • Status changed from assigned to new

Add Comment

Modify Ticket

Action
as new The owner will remain rjollos.
Author


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

 
Note: See TracTickets for help on using tickets.