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

feat(cli): add support for reading schema from stdin in subgraph check command #1512

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+31 −15
Open

feat(cli): add support for reading schema from stdin in subgraph check command #1512

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 31 additions & 15 deletions cli/src/commands/subgraph/commands/check.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ import { BaseCommandOptions } from '../../../core/types/types.js';
import { verifyGitHubIntegration } from '../../../github.js';
import { handleCheckResult } from '../../../handle-check-result.js';

// Helper function to read from stdin
async function readStdin(): Promise<Buffer> {
const chunks: Buffer[] = [];
for await (const chunk of process.stdin) {
chunks.push(Buffer.from(chunk));
}
return Buffer.concat(chunks);
}

export default (opts: BaseCommandOptions) => {
const command = new Command('check');
command.description('Checks for breaking changes and composition errors with all connected federated graphs.');
Expand All @@ -20,25 +29,13 @@ export default (opts: BaseCommandOptions) => {
'--skip-traffic-check',
'This will skip checking for client traffic and any breaking change will fail the run.',
);

command.action(async (name, options) => {
let schemaFile;
let schema: Buffer;

if (!options.schema && !options.delete) {
program.error("required option '--schema <path-to-schema>' or '--delete' not specified.");
}

if (options.schema) {
schemaFile = resolve(options.schema);
if (!existsSync(schemaFile)) {
program.error(
pc.red(
pc.bold(`The readme file '${pc.bold(schemaFile)}' does not exist. Please check the path and try again.`),
),
);
}
}

const { gitInfo, ignoreErrorsDueToGitHubIntegration } = await verifyGitHubIntegration(opts.client);
let vcsContext: VCSContext | undefined;

Expand All @@ -50,8 +47,27 @@ export default (opts: BaseCommandOptions) => {
});
}

// submit an empty schema in case of a delete check
const schema = schemaFile ? await readFile(schemaFile) : Buffer.from('');
// Handle schema input
Copy link
Contributor

Choose a reason for hiding this comment

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

@L42y the apollo approach in accepting the schema from stdin is unusual. Why should we use the --schema flag when piping data is already implicit? Additionally, --schema is declared as a way to specify a file path.

What do you think about this approach?

data | wgc subgraph check my-graph

This would follow the behaviour of common unix tools. An example to implement this is demonstrated here. This can be then also reused across all other commands.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me know what you think, I think both approaches has it's benefits.

if (options.schema) {
if (options.schema === '-') {
// Read from stdin
schema = await readStdin();
} else {
// Read from file
const schemaFile = resolve(process.cwd(), options.schema);
if (!existsSync(schemaFile)) {
program.error(
pc.red(
pc.bold(`The schema file '${pc.bold(schemaFile)}' does not exist. Please check the path and try again.`),
),
);
}
schema = await readFile(schemaFile);
}
} else {
// For delete operations
schema = Buffer.from('');
}

const resp = await opts.client.platform.checkSubgraphSchema(
{
Expand Down
Loading