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

Add session parameters #161

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bbenzikry
Copy link

Hi there, we're testing out the library against a causal cluster with multiple database support.
We didn't find a proper way ( let me know if we just missed something ) to pass session parameters from the basic connection instance - so we currently inherit and add an implementation similar to the one in the PR.

Unfortunately I don't really have the time to add tests etc. for the addition and different session configuration values, but thought it would be a good addition in any case, especially for enterprise users.

@databu
Copy link

databu commented Oct 28, 2020

I'm using a workaround for that: I don't call .run() on the builder but call .buildQueryObject() to get the query object instead, which one can then use to run in a neo4j-driver-created session. That way, one can also use transactions, and generally the driver API.

This is better anyway IMHO, then the query builder can focus on its raison d'être -- building the query -- and doesn't have to meddle with the connection, session and transaction handling, error cases therein etc. Good old separation of concerns.

Example:

const query = new Query().create(...).buildQueryObject();
const session = neo4jDriver.session({ database: database });
return session.writeTransaction(tx => { 
  return tx.run(query.query, query.params)
    .then(...);
}).finally(() => session.close());

@bbenzikry
Copy link
Author

Hi @databu thanks for replying. I'll start by saying we also resorted to using a similar approach ( for constructed queries ) to the one above for transactions specifically. However, I don't necessarily agree it's the main appeal - having a single fluent API to work with for both raw / cypher and constructed queries is also a factor. Separation is nice, but so is abstraction and DX.

@bboure
Copy link
Contributor

bboure commented Jan 4, 2021

I'd like to back this PR, although I would move the session params at the Query level (instead of the Connection level).
That leaves you more flexibility to choose not only the DB, but also the mode (READ/WRITE) depending on your query.

@bbenzikry
Copy link
Author

Hi @bboure, thanks for responding - I think what you mentioned deserves a separate PR for query level overrides.
When we use a lot of queries that are pretty similar, we prefer the connection and session to have sensible and configurable defaults. In our case, we pass a dataloader between graphql resolvers and having the session preconfigured to the right mode / db etc. completely abstracts away handling properties that are not related to logic IMHO.

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.

3 participants