Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#922 closed task (complete)

Callback for foreign module configuration

Reported by: vorner Owned by: vorner
Priority: medium Milestone: Sprint-20110517
Component: configuration Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 2.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This is continuation of #875. This will allow registration of callback whenever configuration of foreign module changes. This is needed to update the keyring every time user changes the list of known TSIGs, but will be potentially useful for other things as well in future.

Subtickets

Change History (7)

comment:1 Changed 9 years ago by vorner

  • Owner changed from vorner to UnAssigned
  • Status changed from new to reviewing

It should be ready for review now. It is possible to:

  • Add a remote config using it's name instead of it's spec filename (who should search for it where it is installed, right?).
  • Register a handler function which is called whenever the remote configuration changes.

As this is backward compatible extension of current API, I don't think it needs a changelog entry, users wouldn't care.

comment:2 Changed 9 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:3 follow-up: Changed 9 years ago by jelte

  • Owner changed from jelte to vorner

A few comments for now, and I have a few i want documented somewhere so as not to forget.

Think this was already a problem, but easy to remedy now; we should move the subscribe() call down until after the last possible exception (or unsubscribe() before we throw) in the add_remote() call, so that we don't end up with 'loose' subscription if something goes wrong in the setup.

Do we have/want a strict requirement that module names don't contain dots? They can't contain / due to another implementation limitation right now. If we do not have or want such a requirement, perhaps we should have two separate methods for adding them either with a module name or a file name. (Also, not necessarily a problem, but we should be aware of it: using a module name instead of a specfile means that said module has to be running (so that it is known for config manager) right now. Hopefully we can make this problem disappear soonish :)))

Can you add a bit of text to the documentation that the handler should never throw? (or, alternatively, catch any exception when calling it, so that update and init do not bail out should the handler throw an exception)

And a few general comments that we should probably create tickets for:

There is a separate python implementation for ccsession etc. we should add this callback functionality there too (can be a separate ticket so as not to block the other tickets that depend on this one).

This was already a potential problem, and solving it will probably require some internal protocol change, but if an updated configuration is rejected by a module, I don't think the modules that are peeking in on its config values are notified of this. Same as for previous point, as this was already a problem, we should probably make this a ticket.

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

  • Owner changed from vorner to jelte

Hello

Replying to jelte:

Think this was already a problem, but easy to remedy now; we should move the subscribe() call down until after the last possible exception (or unsubscribe() before we throw) in the add_remote() call, so that we don't end up with 'loose' subscription if something goes wrong in the setup.

OK, done.

Do we have/want a strict requirement that module names don't contain dots? They can't contain / due to another implementation limitation right now. If we do not have or want such a requirement, perhaps we should have two separate methods for adding them either with a module name or a file name. (Also, not necessarily a problem, but we should be aware of it: using a module name instead of a specfile means that said module has to be running (so that it is known for config manager) right now. Hopefully we can make this problem disappear soonish :)))

I added a bool parameter. It is less duplicate code and the change was smaller.

Can you add a bit of text to the documentation that the handler should never throw? (or, alternatively, catch any exception when calling it, so that update and init do not bail out should the handler throw an exception)

OK

There is a separate python implementation for ccsession etc. we should add this callback functionality there too (can be a separate ticket so as not to block the other tickets that depend on this one).

I didn't need it right now, this is taken as a part of something else O:-). But OK, I'll create a ticket.

This was already a potential problem, and solving it will probably require some internal protocol change, but if an updated configuration is rejected by a module, I don't think the modules that are peeking in on its config values are notified of this. Same as for previous point, as this was already a problem, we should probably make this a ticket.

Well, I wrote some notes about this into some ticket already. I'd like to move all checking of configuration into the python plugins, which would solve two things:

  • The problem with offline configuration.
  • The check is called now first and no notification is done if there's an error. We could even do some kind of transaction ‒ first check everything and then merge and notify. If one fails, all fail.

Anyway, could you have a look at the new changes?

Thanks

comment:5 Changed 9 years ago by jelte

  • Owner changed from jelte to vorner

Changes look good, this can be merged.

ps. Not sure about whether *all* checks should get python plugins (though not necessarily opposed right now), but we do indeed need transactions or a three-phase commit

comment:6 Changed 9 years ago by vorner

  • Resolution set to complete
  • Status changed from reviewing to closed

comment:7 Changed 9 years ago by vorner

  • Estimated Difficulty changed from 0.0 to 2
Note: See TracTickets for help on using tickets.