-
Notifications
You must be signed in to change notification settings - Fork 8
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 support for multiple clients #15
Comments
The function const pgKnex = Knex({
client: 'pg',
connection: process.env.PG_CONNECTION_STRING,
});
const myKnex = Knex({
client: 'mysql',
connection: process.env.MY_CONNECTION_STRING,
});
const models = createSqlmancerClient('./src/**/*.graphql', {
pg: pgKnex
mysql: myKnex
}); In order to specify each model's connection, the type Actor @model(db: "pg", table: "actor", pk: "id") {
id: ID!
firstName: String!
lastName: String!
}
type Movie @model(db: "mysql", table: "movies", pk: "id") {
id: ID!
name: String!
year: Number!
genre: MovieGenreEnum!
} Therefore : models.Actor.knex === pgKnex; // true
models.Movie.knex === myKnex; // true |
@yanickrochon I think the above suggested changes to the API make perfect sense. However, I don't think consumers of the API should have to go through a particular model to access the Knex instance. In other words, I should be able to do:
instead of
So the typing would look something like:
If a single Knex instance is passed in instead of a map, we can default to using I'm not opposed to exposing the underlying Knex instance as a property on each individual model -- but I don't think the relationship between the connection and its models should be completely inverted. |
Sure! The point was to expose each database connections. So, what you're suggesting is fine with me also. But I fail to see why const db = {
pg: pgKnex
mysql: myKnex
};
const client = createSqlmancerClient('./src/**/*.graphql', db);
// client.db === db What is actually relevant are the models. Something like : const db = {
pg: pgKnex
mysql: myKnex
};
const models = createSqlmancerClient('./src/**/*.graphql', db);
export { db, models }; In short, whatever |
@yanickrochon Just to provide some context: the "client" returned by We could just have a |
However, the current design, along with issue #122, introduces a chicken and the egg paradox, namely that creating the models from an already defined schema, including the
This is why I was suggesting keeping the models inside an object exposed by SQL Mancer, something like // resolver.js
import { models } from 'sqlmancer';
export default {
Query: {
films: (root, args, ctx, info) => models.Film.findMany().resolveInfo(info).execute()
}
}; // setup.js
const typeDefs = loadTypeDefs();
const resolvers = loadResolvers();
const schema = makeSqlmancerSchema({ typeDefs, resolvers });
const db = { ... };
const client = createSqlmancerClient(schema, db); However, one way of simplifying this would be to change the way this function is called, and provide a way to pass optional arguments, for example interface SqlmancerClientOptions {
glob: String,
db: Knex | Object<String,Knex>,
typeDefs: Object,
resolvers: Object,
schema: Object
}
function createSqlmancerClient<T extends GenericSqlmancerClient = GenericSqlmancerClient>(options: SqlmancerClientOptions);
|
The
@sqlmancer
directive could accept a list configuration objects instead of just one, allowing the generation of multiple database clients. This would be useful for projects that utilize more than one database. Aname
option would be necessary to distinguish between clients and identify which client (or clients) a particular model belongs to.The text was updated successfully, but these errors were encountered: