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.

Sign up for GitHub

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

Create Database Command #435

Merged
merged 6 commits into from
Nov 25, 2024
Merged

Create Database Command #435

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e18006f
setup create database
henryfauna Nov 23, 2024
59e0d13
add tests and types
henryfauna Nov 23, 2024
fc5350c
setup error handling for common errors
henryfauna Nov 25, 2024
68ed54a
cleanup
henryfauna Nov 25, 2024
5906b68
appease linter
henryfauna Nov 25, 2024
61edbd9
address commnents
henryfauna Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/commands/database/create.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,46 @@
//@ts-check

import { fql } from "fauna";
import { container } from "../../cli.mjs";
import { runV10Query } from "../../lib/fauna.mjs";
import { commonQueryOptions } from "../../lib/command-helpers.mjs";

async function createDatabase() {
async function createDatabase(argv) {
const logger = container.resolve("logger");
logger.stdout(`TBD`);

await runV10Query({
url: argv.url,
secret: argv.secret,
query: fql`Database.create({
name: ${argv.name},
protected: ${argv.protected ?? false},
typechecked: ${argv.typechecked ?? false}
})`,
});

logger.stdout(`Database ${argv.name} created`);
}

function buildCreateCommand(yargs) {
return yargs
.options({
...commonQueryOptions,
henryfauna marked this conversation as resolved.
Show resolved Hide resolved
name: {
type: "string",
description: "the name of the database to create",
},
typechecked: {
type: "boolean",
description: "enable typechecking for the database",
},
protected: {
type: "boolean",
description: "allow destructive schema changes",
},
priority: {
type: "number",
description: "user-defined priority assigned to the child database",
},
})
.demandOption("name")
.version(false)
Expand Down
31 changes: 31 additions & 0 deletions src/lib/fauna.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
export const getV10Client = async ({ url, secret }) => {
const { Client } = (await import("fauna")).default;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, i totally lied to you about there not being an existing v10 client usage in the project :(

the eval command has its own v10 query path that's used by it and the shell command:
https://github.com/fauna/fauna-shell/blob/v3/src/commands/eval.mjs#L101

i think it would make the most sense for eval and shell to only have the formatting logic, and for us to have only one implementation of the client/network stack. take a look at getSimpleClient and performQuery and see what you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that v10 path is not using the driver though so that's why I set up these new helpers. I definitely agree about having a single implementation, I tried consolidating things a bit but it was proving to be difficult. Something to sort out next week :)

return new Client({
secret,
endpoint: url,
});
};

export const runV10Query = async ({
query,
url = undefined,
secret = undefined,
client = undefined,
options = {
format: "simple",
typecheck: false,
},
}) => {
let _client = client;
echo-bravo-yahoo marked this conversation as resolved.
Show resolved Hide resolved

if (!_client && url && secret) {
_client = await getV10Client({ url, secret });
} else if (!_client) {
throw new Error("No client provided and no url and secret provided");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about pushing the validation all the way down to getV10Client? there's validation at the very top of the stack (argv); might make more sense for this validation to live at the very bottom of the stack and let the middle parts assume valid input.

Copy link
Contributor Author

@henryfauna henryfauna Nov 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can push it to getV10Client. What's the downside of doing validation in runV10Query?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept a bit of validation in runV10Query just to sanity check the args - let me know if you think there's a better way to do this though

}

return _client.query(query, options).finally(() => {
// Clean up the client if one was created internally.
if (!client) _client.close();
});
};
Loading