-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement dataconnect:execute command. #9274
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
Implement dataconnect:execute command. #9274
Conversation
Created using spr 1.3.6-beta.1
}); | ||
} | ||
|
||
// Log the body to stdout to allow pipe processing (even with .errors). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work quite nicely since errors thrown (below, if any) are also logged to stdout (globally handled). Piping to jq
for example can choke on the extra output
src/commands/dataconnect-execute.ts
Outdated
} else { | ||
if (fileOrConnector === "-") { | ||
stdinUsedFor = "operation source code"; | ||
if (process.stdin.isTTY) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed internally, I tried to throw an error if options.nonInteractive
but that creates a false positive when --json
is passed, since --json
right now implies options.nonInteractive
(handled globally).
I don't think erroring on --json
matches user intention, so I left that part out.
Can you limit the scope of this PR to a minimum set of API that has no ongoing debates? Probably will make it easier to make progress and unblock the primary seed data CUJ in quick start.
|
Created using spr 1.3.6-beta.1
Removed connector and |
Created using spr 1.3.6-beta.1
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This PR introduces a new firebase dataconnect:execute
command, which is a great addition for developers working with Data Connect. The implementation is comprehensive, covering various ways to specify queries and variables, including from files and stdin, which improves usability.
I've left a couple of suggestions to improve error handling in JSON parsing functions. Making error messages more precise will help users debug their inputs more effectively.
Overall, this is a solid contribution that enhances the Data Connect developer experience.
Created using spr 1.3.6-beta.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small things and a general concern about the deeply nested code. If feasible, it would be great to simplify a bit, but if not, I won't block on it.
return source.files[0].content; | ||
} | ||
let query = ""; | ||
for (const f of source.files) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highly tempted to do this with functional programming as .filter(...).map(...).reduce(...), but this probably reads cleaner as is.
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Description
This adds a new command to execute Data Connect operations.
Scenarios Tested
See Sample Commands below for more.
Sample Commands
firebase dataconnect:execute MyOperation # REMOVED for now: firebase dataconnect:execute my-connector MyOperation firebase dataconnect:execute ./my-file.gql firebase dataconnect:execute ./my-file.gql MyOperation