If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

mozsvc is unable to create plugin instances when konfig extensions are used ("extends" / "overrides")

RESOLVED FIXED

Status

Cloud Services
Server: Sync
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
When configuring the sync-server, with the ini file being splitted in 2 parts:

> [DEFUALT]
> overrides = /etc/syncserver-private.ini

I see the following error message:

>   File "/nix/store/0nhhz0khzcvxyq9jrix05lr2w4p81das-python2.7-mozsvc-0.8/lib/python2.7/site-packages/mozsvc/plugin.py", line 85, in load_and_register
>     plugin = load_from_settings(section_name, settings)
>   File "/nix/store/0nhhz0khzcvxyq9jrix05lr2w4p81das-python2.7-mozsvc-0.8/lib/python2.7/site-packages/mozsvc/plugin.py", line 132, in load_from_settings
>     return klass(**kwargs)
>   File "/nix/store/yg36b4j479mqhswx7b9j5swp2phhh9b1-python2.7-tokenserver-1.2.11/lib/python2.7/site-packages/tokenserver/verifiers.py", line 50, in __init__
>     super(LocalVerifier, self).__init__(**kwargs)
> TypeError: __init__() got an unexpected keyword argument 'overrides'

The issue is caused by the fact that konfig / configparser is adding the DEFAULT keys to all other sections.  As they are added to all sections, when we have a the following configuration, then an extra overrides item is added

> [browserid]
> backend = tokenserver.verifiers.LocalVerifier
> audiences = http://localhost:5000
> overrides = /etc/syncserver-private.ini

As these items are not part of the list of arguments expected by the LocalVerifier, these are raising a TypeError, as seen above.
(Assignee)

Comment 1

3 years ago
Created attachment 8535200 [details] [review]
Filters out konfig keywords from the settings to create plugins.
Attachment #8535200 - Flags: review?(rfkelly)
Comment on attachment 8535200 [details] [review]
Filters out konfig keywords from the settings to create plugins.

LGTM; we might consider adding a simple test but the change is so small and obvious that it hardly seems worth it.  (The github PR failed in Travis, but the failure appears unrelated to your change)
Attachment #8535200 - Flags: review?(rfkelly) → review+
I'm not super clear on how bugzilla<->github-pr reviews are supposed to work; I guess you update the commit with r=me and I'll merge it?
(Assignee)

Comment 4

3 years ago
(In reply to Ryan Kelly [:rfkelly] from comment #3)
> I'm not super clear on how bugzilla<->github-pr reviews are supposed to
> work; I guess you update the commit with r=me and I'll merge it?

I am not either, but this is how b2g guys are working as far as I know.
I already updated the commit, so you can merge it.
https://github.com/mozilla-services/mozservices/commit/f86c0b0b870cd8f80ce90accde9e16ecb2e88863
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

3 years ago
(In reply to Ryan Kelly [:rfkelly] from comment #2)
> LGTM; we might consider adding a simple test but the change is so small and
> obvious that it hardly seems worth it.  (The github PR failed in Travis, but
> the failure appears unrelated to your change)

I do not yet have all the dependencies packaged for running the tests, but I tested locally with the sync server, and the issue is fixed by this patch.
You need to log in before you can comment on or make changes to this bug.