Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension Context #109

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Extension Context #109

wants to merge 16 commits into from

Conversation

kpelzel
Copy link
Contributor

@kpelzel kpelzel commented Nov 4, 2019

Problem:
Some extensions are only discovered by the parent node. Currently if the parent node makes a discovery event after the node has gone into sync state, the next incoming hello packet will overwrite all extensions regardless of anything the parent node discovers. This is due to the source of truth for discoverable state coming from the child nodes.

Solution:
Here's a proposal for a simple solution. Each extension now has a "Context" method which is either:

  • lib.ExtensionContext_PARENT
  • lib.ExtensionContext_CHILD

If an extension has a context of PARENT, that extension will be removed from any hello packets that are coming from the child nodes because that extension can only be discovered by the parent. Anything with a child context will not be removed.

(Also, ignore my terrible commit messages)

@jlowellwofford
Copy link
Contributor

We may want to be able to specify the context on a per-variable level. This would probably require the ability to add metadata to variables, which adds a bit of complexity, but could be used for other things as well.

Needs further discussion.

@jlowellwofford
Copy link
Contributor

I would like to revisit this. While it may be nice to have per-variable support, I don't think that excludes per-extension support.

@kpelzel would you mind rebasing this to a modern commit and making sure it still works?

Base automatically changed from master to main March 23, 2021 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants