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

Make DB add/remove more flexible #199

Open
brianru opened this issue Sep 12, 2019 · 7 comments
Open

Make DB add/remove more flexible #199

brianru opened this issue Sep 12, 2019 · 7 comments

Comments

@brianru
Copy link
Contributor

brianru commented Sep 12, 2019

The following code restricts the accepted query params to graph-uri. That is the only supported param, at the moment. But the inflexibility means each new param requires a stardog.js change. Here is a Stardog issue for a new param: https://github.com/stardog-union/stardog/issues/7816

const queryParams = {};
if (params.graphUri) {
queryParams['graph-uri'] = params.graphUri;
}

@jmrog
Copy link
Contributor

jmrog commented Sep 12, 2019

The following code restricts the accepted query params to graph-uri. That is the only supported param, at the moment. But the inflexibility means each new param requires a stardog.js change. Here is a Stardog issue for a new param: stardog-union/stardog#7816

const queryParams = {};
if (params.graphUri) {
queryParams['graph-uri'] = params.graphUri;
}

This is a good point, but flexibility also has costs. In the v2-typescript branch, I was considering making it so that the types of the params would be inferred, so that consumers of stardog.js could know which params are accepted and which are not (via types/diagnostics). Sounds like maybe you wouldn't like that?

@jmrog
Copy link
Contributor

jmrog commented Sep 12, 2019

Example: https://github.com/stardog-union/stardog.js/blob/v2-typescript/lib/db/main.ts#L26

Currently, TypeScript can flag errors when you supply params that are unexpected. Does that seem too inconvenient to be of benefit? It's pretty nice to not have to look at another codebase when writing code consuming stardog.js.

@brianru
Copy link
Contributor Author

brianru commented Sep 12, 2019

Am I wrong to think that a user of stardog.js could then ignore the error? Or, could the type simply be open to other stuff so it validates known params but doesn't restrict the user from using others?

@brianru
Copy link
Contributor Author

brianru commented Sep 12, 2019

For this issue...I really just wanted to flag that at some point support for removeAll will have to be added.

@jmrog
Copy link
Contributor

jmrog commented Sep 12, 2019

Am I wrong to think that a user of stardog.js could then ignore the error? Or, could the type simply be open to other stuff so it validates known params but doesn't restrict the user from using others?

Yeah, something along these lines seems reasonable.

@brianru
Copy link
Contributor Author

brianru commented Sep 12, 2019

We sit on different sides of the flexibility / dev experience fence :)

@jmrog
Copy link
Contributor

jmrog commented Sep 16, 2019

Let's wait and see (for now, at least) whether a user of stardog.js (other than ourselves) wants this.

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

2 participants