Rework eval and shell to fully implement eval #443

ecooper · 2024-11-26T21:52:09Z

Problem

The current implementation of eval was not fully ported over, and recent CLI changes have started to add some drift in how we interact with Fauna.

Solution

This is a "big" PR that's really pretty simple:

Renames the command to query but enables an eval alias for the old-timers
Moves all query helpers out of the query command and into fauna.mjs and faunadb.mjs. Each module implements the following:
- stringExpressionToQuery: Transform a string into a query
- getClient: to return a client
- runQuery: execute a query
- runQueryFromString: helper to run a query directly from a string input
- formatError: support error formatting in a single place
- formatQueryResponse: support driver response formatting in a single place
Use fauna-client for a single entry point to make queries:
- runQueryFromString: wraps the version specific implementations
- formatError: wraps the version specific implementations
- formatQueryResponse: wraps the version specific implementations
The following flags have been added:
- extra: returns the full API response instead of just the data. This flag is surfaced in the shell via .toggleExtra
- quiet: mutes stderr
- apiVersion: version has been renamed to not conflict with cli version
The following flags have been removed:
- format: When using the driver, format isn't specifically helpful. The undocumented decorated format doesn't add enough value to manage separately. If we want colorized JSON output (or something else altogether), plenty of options exist.

Result

  fauna query "Collection.all()" --database us-std/example --role admin                                   run the query and write to stdout
  fauna query -i /path/to/queries.fql --database us-std/example --role admin                              run the query from a file
  echo "1 + 1" | fauna query - --database us-std/example --role admin                                     run the query from stdin
  fauna query -i /path/to/queries.fql -o /tmp/result.json --database us-std/example --role admin          run the query and write to a file
  fauna query -i /path/to/queries.fql -o /tmp/result.json --extra --database us-std/example --role admin  run the query and write full API response to a file
➜  fauna-shell (v3-eval-to-query)

Testing

See query tests.

Next/Out of scope

Apply proper stderr/stdout patterns in other commands
Update shell tests
Explore other output formats/styling for query when --json is not specified

echo-bravo-yahoo · 2024-11-26T22:38:07Z

src/commands/query.mjs

+    const output = formatQueryResponse(results, argv);
+
+    if (argv.output) {
+     container.resolve("fs").writeFileSync(argv.output, output);


do we want to output the result even when we write to file? sort of like tee.

I see -i and -o as helpers for pipe/redirection so my instinct would to redirect output to the file. But if it makes sense to enough people to do both, that's fine too.

echo-bravo-yahoo · 2024-11-26T22:40:53Z

src/commands/query.mjs

+      type: "string",
+      description: "the query to run; use - to read from stdin",
+    })
+    .nargs('query', 1)


this'll probably cause issues with queries that have spaces in them. is the behavior better if we don't specify a number and let yargs infer? or do we want to enforce string quoting?

see queries like:

fauna query let x = 5 --no-color

Ah, yeah. Good call out. Let me putz around with it and see.

This usage came from the yargs parser tests, but those were also operating on single tokens/values.

Well, trying to write FQL in zsh without quotes is super dangerous...so I might not do anything about this until we get more feedback from folks.

src/commands/shell.mjs

wildemat · 2024-11-28T01:25:10Z

src/commands/query.mjs

+      ['$0 query "Collection.all()" --database us-std/example --role admin', "run the query and write to stdout "],
+      ["$0 query -i /path/to/queries.fql --database us-std/example --role admin", "run the query from a file"],
+      ['echo "1 + 1" | $0 query - --database us-std/example --role admin', "run the query from stdin"],
+      ['$0 query -i /path/to/queries.fql -o /tmp/result.json --database us-std/example --role admin', "run the query and write to a file"],
+      ['$0 query -i /path/to/queries.fql -o /tmp/result.json --extra --database us-std/example --role admin', "run the query and write full API response to a file"],


--role will default to admin when minting a key for the specified database path. Maybe examples should show you don't have to provide role arg.

Also reminds me to go test out custom user roles...

Are you sure?

./src/user-entrypoint.mjs query -d us-std/A --url https://db.fauna-dev.com "Database.all()" fauna query [fql] <...snip> Failed to make request to Fauna account API [404]: undefined - Role 'undefined' doesn't exist.

wildemat · 2024-11-28T01:28:24Z

src/lib/fauna-client.mjs

+      const credentials = container.resolve("credentials");
+
+      await credentials.databaseKeys.onInvalidCreds();
+      const refreshedSecret = await credentials.databaseKeys.getOrRefreshKey();


Since we already awaited the refresh via onInvalidCreds, safe to just access ".key" here

I'll clean this up in a follow-up.

wildemat · 2024-11-28T01:32:24Z

src/lib/faunadb.mjs

+const validateQueryParams = ({ query, client, url, secret }) => {
+  if (!query) {
+    throw new Error("A query is required.");
+  } else if (!client && (!url || !secret)) {
+    throw new Error("A client or url and secret are required.");
+  }
+};


Saw some validation of args above somewhere as well. I have some arg validation in Credentials class too.

Thoughts on validating all args and their combinations in one place in middleware before we get to handlers?

In theory, this check doesn't need to be here at all. The fauna client will barf if we're missing things. This error isn't adding a lot of value as-is.

ecooper requested a review from a team November 26, 2024 21:52

echo-bravo-yahoo reviewed Nov 26, 2024

View reviewed changes

E. Cooper added 3 commits November 27, 2024 11:26

Rework eval and shell to fully implement eval as query

36e7b76

Rename query to fql

20f8eba

Update tests so they pass except for shell

56e186d

ecooper force-pushed the v3-eval-to-query branch from ca195e1 to f16b66c Compare November 27, 2024 21:30

Hook up creds for database paths

2024620

ecooper force-pushed the v3-eval-to-query branch from f16b66c to 2024620 Compare November 27, 2024 21:32

E. Cooper added 5 commits November 27, 2024 13:41

Fix query tests

df3606e

Remove old fauna client helpers

eea6628

Resolve linting errors

3c2ffd8

Move common validator to command helpers

b75996d

Update examples to use -d and -r for now

208315d

ecooper requested a review from a team November 27, 2024 22:56

ecooper marked this pull request as ready for review November 27, 2024 22:59

wildemat approved these changes Nov 28, 2024

View reviewed changes

henryfauna approved these changes Dec 2, 2024

View reviewed changes

ecooper merged commit cbdc8cd into v3 Dec 2, 2024
4 checks passed

ecooper deleted the v3-eval-to-query branch December 2, 2024 18:18

This was referenced Dec 2, 2024

login/account keys reference 'user' instead of 'profile' #445

Merged

Credentials Middleware tests #461

Merged

cleve-fauna mentioned this pull request Dec 5, 2024

v3 fixClientId #478

Closed

This was referenced Dec 6, 2024

login test fix #492

Merged

customttl #503

Closed

cleve-fauna mentioned this pull request Dec 13, 2024

v3 colorLogs #523

Closed

wildemat mentioned this pull request Dec 18, 2024

update driver #543

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework eval and shell to fully implement eval #443

Rework eval and shell to fully implement eval #443

ecooper commented Nov 26, 2024 •

edited

Loading

echo-bravo-yahoo Nov 26, 2024

ecooper Nov 27, 2024 •

edited

Loading

echo-bravo-yahoo Nov 26, 2024

ecooper Nov 27, 2024

ecooper Nov 27, 2024

wildemat Nov 28, 2024

ecooper Dec 2, 2024

wildemat Nov 28, 2024

ecooper Dec 2, 2024

wildemat Nov 28, 2024

ecooper Dec 2, 2024

Rework eval and shell to fully implement eval #443

Rework eval and shell to fully implement eval #443

Conversation

ecooper commented Nov 26, 2024 • edited Loading

Problem

Solution

Result

Testing

Next/Out of scope

Choose a reason for hiding this comment

ecooper Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecooper commented Nov 26, 2024 •

edited

Loading

ecooper Nov 27, 2024 •

edited

Loading