Modify

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#7380 closed defect (invalid)

Dummy add_domain() in api.py needs to accept arguments

Reported by: JirkaV Owned by: doki_pen
Priority: normal Component: AnnouncerPlugin
Severity: normal Keywords: i18n backwards-incompatible
Cc: hasienda Trac Release: 0.11

Description

Hi,

first, thanks for an awesome plugin!

I just upgraded the plugin on my site and hit a small bug.

The add_domain() function for Trac 0.11 compatibility does not accept arguments, but it's called with 2 arguments below. Trac 0.11 fails with an internal error (surprise! :)

Changing the dummy function to add_domain(args, *kwargs) solves the issue.

Thanks again!

Jirka

Attachments (0)

Change History (13)

comment:1 Changed 4 years ago by rjollos

  • Summary changed from dummy add_domain() in api.py needs to accept arguments to Dummy add_domain() in api.py needs to accept arguments

Can you tell us exactly which 0.11.x version of Trac you are using? And, did you install from announcerplugin/0.11?

comment:2 Changed 4 years ago by JirkaV

Actually, no. The problem is in trunk (I don't know why I originally installed that one). The bug is still there as of r8361.

comment:3 follow-up: Changed 4 years ago by rjollos

  • Cc hasienda added

I'm not sure the current trunk will work with Trac 0.11. I think it is being developed for 0.12. hasienda might know better though. You may want to use 0.11 or 0.11dev branches.

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

  • Keywords i18n backwards-incompatible added
  • Resolution set to invalid
  • Status changed from new to closed

Replying to rjollos:

I'm not sure the current trunk will work with Trac 0.11. I think it is being developed for 0.12. hasienda might know better though. You may want to use 0.11 or 0.11dev branches.

Right, this is definitely not working, sorry.

AFAIK trunk is commonly meant to be bleeding edge, so this is (at least) 0.12. With introduction of i18n basics and translations there is no way to compile with anything but Babel and a late Trac 0.12dev, 0.12 or 0.13dev installed, to satisfy vital dependencies like i.e. add_domain, that doesn't make sense in a non-i18n environment at all.

BTW, by no means this is a 'dummy function', as you suggested, but a convenience module provided in later Trac to bind plugins translation in a sane way and distinct to other message catalogs, even Trac's own one.

Another question is, of it's worth to aim at a compatibility branch 0.11/0.12 without i18n. While it could be done (did so with WikiTicketCalendarMacro recently, this is definitely a big effort. As we now have much more urgent development goals (at least I think so with the proposal for integration into Trac), this might not happen and may be getting degrading attention by progressing migration to current stable 0.12.

Any suggestions, contributions included, are very welcome, even to 0.11.

But don't dare to deactivate features in trunk, that can't work with older Trac releases. This is clearly an invalid approach.

comment:5 Changed 4 years ago by rjollos

On trunk, the following parameter in setup.py should probably be changed to at least 0.12dev:

 install_requires = [ 'trac>=0.11' ],

comment:6 Changed 4 years ago by anonymous

Oh, sure, thanks for the hint. I'll change this with one of the next commits.

comment:7 Changed 4 years ago by hasienda

^^ that was me. (Hope to get 0.12 here really soon, so I could comment/change comments.)

comment:8 Changed 4 years ago by JirkaV

OK, thanks for the update! I think we're talking on different frequencies here, not hearing to each other. I'll try to be more clear:

  • I think the bug I opened only describes a syntax error in a specific edge case. I may be the only one who hit the situation. If it doesn't get fixed, that's fine with me.
  • FWIW, the current trunk does work with Trac 0.11 (at least for me).
  • the problem I was trying to describe is an simple syntax error with a simple fix. I'm not trying to cover anything else.
  • The problem is following:
    • in some specific situations (Trac 0.11), the add_domain() function is not available from Trac itself so a dummy version is defined in api.py here. This "dummy" function accepts no parameters
    • the function is called during init() here. It uses parameters so it raises SyntaxError if the previously defined "dummy" function is used instead of the one normally provided by Trac.

That's all. I was only suggesting that the dummy function is modified to accept (*args, kwargs) to avoid the syntax error when its used.

Cheers

Jirka

comment:9 Changed 4 years ago by JirkaV

Hmm, seems that Trac urlencoded the links so these are unusable now. Anyway, the "dummy" function I was mentioning all the time is this definition in api.py itself:

except ImportError:
    # fall back to 0.11 behavior, i18n functions are no-ops then
    def add_domain():
        pass

comment:10 follow-up: Changed 4 years ago by hasienda

Well, my apologies, if it came out rude, certainly wasn't intended on my side.

I've already discussed with doki_pen and agreed to create a dedicated 0.12 branch now. This should clarify things considerably. I've never tested trunk with 0.11, but nice, if it's still working well. This feedback is definitely appreciated.

I still don't get the dummy function thing, since I neither see nor use it that way but it's definitely needed to get i18n initialized. Anyway you're focussed at 0.11 without i18n and this is fine as well. I invite you to follow development and continue to give your comments, and maybe contribute, if your time permits.?

comment:11 Changed 4 years ago by hasienda

(In [8409]) AnnouncerPlugin: Change dependencies for trunk according to recent development, refs #7380.

comment:12 in reply to: ↑ 10 Changed 3 years ago by hasienda

Replying to hasienda:

I still don't get the dummy function thing, ...

Meanwhile I've got it, see my comment to #7314. This will get fixed soon, and sorry for the inconvenience so far.

comment:13 Changed 3 years ago by hasienda

(In [10920]) AnnouncerPlugin: Refactor i18n in a more compatible way, refs #7314, #7666, #7380, #8062 and #9192.

Lessons learned meanwhile with other plugins, so the code applied here has
actually been tested and proven to work. This reverts [8409] effectively,
but let's keep proper Babel and Trac versions for i18n explicitly as an extra.

After these changes the plugin finally loads cleanly in Trac 0.11 again.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from doki_pen. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.