Skip to content
This repository has been archived by the owner on Dec 2, 2020. It is now read-only.

Multi-tenanancy #75

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Multi-tenanancy #75

wants to merge 17 commits into from

Conversation

trotter
Copy link
Contributor

@trotter trotter commented Jun 25, 2012

I've been working lately on adding some multi-tenancy to cube and would love to get feedback on the approach. I've modified the server config to take two additional parameters authentication and namespace, which let you authenticate incoming requests and namespace event types per requests. You can see an example of authentication in test/authenticated-collector-test.js. I haven't added a test for the namespacing yet, because it involves checking mongo to verify that the type was namespaced. Unfortunately, I don't really know how to test that with node/vows. Any help there would be appreciated.

So... what do you think of this approach? Prefer something else? Think this is interesting?

@trotter
Copy link
Contributor Author

trotter commented Jun 28, 2012

So... everything seems to work now, but the code is quite a mess. @mbostock, you have time to talk about this sometime? I'm happy to come up to SF.

@sbuss
Copy link

sbuss commented Jul 7, 2012

I really like these namespace changes. Thanks for writing this, @trotter

I have a style nit, though. The authentication checking pattern

} catch (e) {
    if (e.toString() == "AuthenticationError: Invalid Credentials") {
        response.writeHead(401, headers);
        response.end(JSON.stringify({error: e.toString()}));
    } else {
        response.writeHead(400, headers);
        response.end(JSON.stringify({error: e.toString()}));
    }
    return;
}

and the namespace application pattern

// Namespace the type if necessary.
if (namespaceFun) {
  namespaceFun(typeWithoutNamespace, request, namespaceFunCallback);
} else {
  namespaceFunCallback(typeWithoutNamespace);
}

function namespaceFunCallback(type) {

are repeated several times and seem like they could be factored out.

Is there a reason that the authentication doesn't just wrap endpoint? Maybe add an authenticated parameter to endpoint that does this authentication checking in one place.

@trotter
Copy link
Contributor Author

trotter commented Jul 10, 2012

Thanks, @sbuss. I'll see about factoring those out. I hadn't actually considered just wrapping it within the server code itself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants