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

Global Validators #58

Open
Stebalien opened this issue Jan 18, 2018 · 8 comments
Open

Global Validators #58

Stebalien opened this issue Jan 18, 2018 · 8 comments

Comments

@Stebalien
Copy link
Member

TODO in #55.

Instead of registering per-topic validators as we do in #55, validators should decide whether or not they care about a topic when we first subscribe to it. E.g.,

type Validator interface {
  Validates(topic string) (bool, error)
  // ...
}
@whyrusleeping
Copy link
Contributor

I don't think the validator should pick which topics it validates. I think we should register a validator for a given topic (or set of topics).

@vyzo
Copy link
Collaborator

vyzo commented Jan 19, 2018

What do we gain by having Validators select the topic they validate?
They complicate the interface, both in terms of writing the validator (can't just pass a function) and the validator selection logic internally.

@Stebalien
Copy link
Member Author

Stebalien commented Jan 19, 2018 via email

@vyzo
Copy link
Collaborator

vyzo commented Jan 19, 2018

For one, multiple subscribers (i.e., pubsub as a service). Otherwise, if we have two subscribers, one will fail to register its validator. I'd expect validators to work like DHT record validators (you register your validators up-front).

I don't understand why that's different from the per-topic validator implementation.
We register the validators once, orthogonal with subscriptions -- if we are to register a validator with every subscription then why did we ditch per-subscription validators?

Also, garbage collection. If I register a validator for each individual topic, I can't garbage collect them even if I unsubscribe.

That would still be true with global validators.

@vyzo
Copy link
Collaborator

vyzo commented Jan 19, 2018

Per discussion on irc, the big gain from global validators would be validator classes.
By using namespaced topics we can register validators for an entire class of topics (eg IPNS) and not have to register a validator for every topic.

@Stebalien
Copy link
Member Author

I've been trying to use validators in IPNS over pubsub and this issue is making it very difficult to do so reliably. At the moment, I can:

  1. Assume that the duplicate (already registered) validator is correct and ignore the error (insecure).
  2. Validate records twice, once in the topic validator and once when processing the value (inefficient).
  3. Track which topics I've already registered validators on (painful).

(and none of these options work in the presence of multiple subscribers)

@aschmahmann
Copy link
Contributor

I've been talking with @Stebalien about this a bit in the context of #184, which has necessitated that we take another look at the PubSub interfaces.

There's a current proposal to shift from PubSub functions like Publish(topic string, ....) to something like topic := JoinTopic(topic string, ...); topic.Publish(...). This has the nice feature of helping separating out the code a bit (global PubSub vs Topic specific operations), and also gives us the ability to deprecate the older interfaces and make any improvements we need to (e.g. adding a context to Publish).

One issue that's come up in the new interfaces is how to deal with Validators and generally if there should exist one shared Topic object per topic, or whether there may exist many handlers for a given topic. If there is only one shared Topic object then it's very easy to make it clear that dealing with Validators is to be handled in application space (e.g. you can only pass a validator in on topic join, so if your application will try to utilize the topic in multiple places make sure they agree on a validator, or alternatively pass in a stateful validator that is extenally updateable). However, if there can be multiple handlers for a given topic then should come to some decisions as to what it would mean for two handlers to each want different validators and how we should deal with that (e.g. have the sum validator be the ORs of all the registered validators for a given topic at the network level and use the specific handlers validator when returning new messages).

@raulk @Stebalien @vyzo Any thoughts you have on this (or the #184 PR in general) would be very helpful.

@Stebalien
Copy link
Member Author

I've created a new issue (#198) to discuss this as it's a bit off topic here. I've also tried to fill in the background a bit so anyone joining the discussion can better understand the proposal and motivation.

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

No branches or pull requests

4 participants