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

feat: namespace configuration #324

Merged
merged 12 commits into from
Nov 26, 2020
Merged

feat: namespace configuration #324

merged 12 commits into from
Nov 26, 2020

Conversation

zepatrik
Copy link
Member

Related issue

contributes to #303

Proposed changes

Adding a namespace config schema and required interfaces.

Further comments

The namespace names can be changed while the id always is fixed.

@zepatrik
Copy link
Member Author

I would like to first merge #315 and adjust the query format a bit @robinbraemer
With this PR I realized that the namespace always has to be present in queries, even if the object ID is not included. Can you please add that in your PR?

@robinbraemer
Copy link
Contributor

robinbraemer commented Nov 18, 2020

I assume this is because the server needs the tuples table name of the namespace, right?

I will add a comment in the ListRequest that the namespace in object is required.

@zepatrik
Copy link
Member Author

Exactly, maybe you even want to split object into ObjectID and Namespace? Would be a cleaner approach then requiring a nested key in my opinion.

@robinbraemer
Copy link
Contributor

robinbraemer commented Nov 19, 2020

Even if some queries do not specify an ObjectID, an Object represents a "resource/digital object" in a namespace.
I'd prefer not to dissolve the strong relation between an object_id and namespace for Query messages and keep the model self contained, just as in other RPC requests like Check, Write, etc.

Would be a cleaner approach then requiring a nested key in my opinion.

Maybe (or not?), IMO using the Object structure in a Query structure probably helps generally more developers that use and operate on the client side with various Objects from different namespaces (and keep the linking). It allows to just set the object field of the Query to their already locally cached Object, instead of separating out the Object fields object_id, namespace directly into Query fields.

It may be a small design detail, but an important one, that makes users more productive.

@zepatrik
Copy link
Member Author

It is an important one for sure!
Keep in mind that not only objects are bound to a namespace but also relations. In the namespace config you have to define all relations for that namespace.
For that reason I think we should separate out the namespace and object id everywhere and have them as separate parameters. It will be cumbersome for clients to have these nested structures.
Also it is required for consistency between the APIs. Expand will have to have a namespace parameter but no object ID, as we need the namespace for the relation.

@zepatrik
Copy link
Member Author

zepatrik commented Nov 19, 2020

So I rechecked the paper, there seems to be no use case mentioned where a request is missing an object. Still, we should separate object ID and namespace everywhere IMO because the namespace is coupled in the same way to the relation, and therefore it should not be nested as we first drafted.

@robinbraemer
Copy link
Contributor

Good, in this case, I'll remove the Object message so we now define and refer to a namespace and object using a single separate fields everywhere. It's great to find and make the API design as similar as proposed in the paper.

@robinbraemer
Copy link
Contributor

robinbraemer commented Nov 19, 2020

What about <object> ::= <namespace>:<object_id> from 2.1 Relation Tuples in the paper? Is it misleading us? :)

Beside this, maybe we should carefully re-read the Read API and re-design the ListRequest based on use cases needed directly described in the paper? Do you find it appropriate at this stage? I'll try to design a Query strictly based on the use cases said in the first lines of 2.4.1 Read to show how an alternative would look like and then we can evaluate and choose one of the designs.

@zepatrik
Copy link
Member Author

This is really just the grammar of the language used in the paper and does not have to correspond 💯 % to the underlying data structures. Maybe there is also a limitation of zanzibar that every request has to include an object. But this would not be a) easy to grasp for people and b) technically required.

@robinbraemer
Copy link
Contributor

robinbraemer commented Nov 19, 2020

How about renaming the string fields object_id to simply object, now that the Object message is gone?

zepatrik and others added 8 commits November 19, 2020 17:18
# Conflicts:
#	cmd/relationtuple/get.go
#	cmd/serve.go
#	internal/check/engine.go
#	internal/check/engine_test.go
#	internal/check/handler.go
#	internal/expand/engine.go
#	internal/expand/engine_test.go
#	internal/expand/handler.go
#	internal/persistence/memory/relationtuples.go
#	internal/persistence/memory/relationtuples_test.go
#	internal/relationtuple/definitions.go
#	internal/relationtuple/handler.go
#	internal/relationtuple/relationtuple.pb.go
#	internal/relationtuple/relationtuple.proto
#	internal/relationtuple/relationtuple_grpc.pb.go
@zepatrik zepatrik merged commit b94f50d into zanzibar Nov 26, 2020
@zepatrik zepatrik deleted the namespace-config branch November 26, 2020 14:47
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.

2 participants