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.

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(rover): rover subgraph check #110

Merged
merged 1 commit into from
Jan 11, 2021
Merged

Conversation

EverlastingBugstopper
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper commented Dec 3, 2020

rover subgraph check ready to go. it pretty much copies exactly what the existing CLI does.

run the checks, print composition errors if they exist.

if no composition errors, iterate over check results and print a formatted table to stderr. if there are failures, count them and return an error so the CLI exits with a non-zero exit code.

user testing here for folks familiar with checks would be super great :)

@EverlastingBugstopper EverlastingBugstopper changed the title [wip] (blocked) checks [wip] feat: rover graph check Dec 4, 2020
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/check-please branch 2 times, most recently from b128a40 to 257dec6 Compare December 7, 2020 18:25
@JakeDawkins JakeDawkins linked an issue Dec 7, 2020 that may be closed by this pull request
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/check-please branch 2 times, most recently from 8a97553 to 842abe1 Compare December 10, 2020 20:17
@EverlastingBugstopper EverlastingBugstopper changed the title [wip] feat: rover graph check feat: rover graph check & rover subgraph check Dec 10, 2020
@EverlastingBugstopper EverlastingBugstopper added this to the 🐣 0.1.0 milestone Dec 10, 2020
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/check-please branch 2 times, most recently from eaea7f7 to 5dfe6d7 Compare December 11, 2020 19:24
src/command/graph/check.rs Outdated Show resolved Hide resolved
src/command/graph/check.rs Outdated Show resolved Hide resolved
src/command/subgraph/check.rs Outdated Show resolved Hide resolved
src/command/subgraph/check.rs Show resolved Hide resolved
src/command/graph/mod.rs Outdated Show resolved Hide resolved
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/check-please branch 3 times, most recently from 5dc2409 to e84b7cb Compare December 14, 2020 16:49
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/check-please branch 4 times, most recently from 6a0be20 to 9a27a30 Compare December 15, 2020 15:59
@EverlastingBugstopper EverlastingBugstopper changed the base branch from main to avery/graph-check December 15, 2020 18:16
@EverlastingBugstopper EverlastingBugstopper changed the title feat: rover graph check & rover subgraph check feat(rover): rover subgraph check Dec 15, 2020
@EverlastingBugstopper EverlastingBugstopper changed the title feat(rover): rover subgraph check [wip] feat(rover): rover subgraph check Dec 15, 2020
@EverlastingBugstopper EverlastingBugstopper changed the base branch from avery/graph-check to main December 15, 2020 18:27
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/check-please branch 3 times, most recently from 46d2fff to d9f22e8 Compare December 17, 2020 21:18
@EverlastingBugstopper EverlastingBugstopper requested review from JakeDawkins and removed request for ashleygwilliams December 18, 2020 20:54
@EverlastingBugstopper EverlastingBugstopper changed the title [wip] feat(rover): rover subgraph check feat(rover): rover subgraph check Dec 18, 2020
crates/rover-client/src/blocking/studio_client.rs Outdated Show resolved Hide resolved
src/command/subgraph/check.rs Outdated Show resolved Hide resolved
let mut num_failures = 0;
for error in composition_errors {
num_failures += 1;
tracing::error!("{}", &error.message);
Copy link
Contributor

Choose a reason for hiding this comment

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

as a potential future improvement, we could make a table of these errors similar to checks. Composition errors should always be in the format [Service] Type.field? -> Message so maybe we could parse out into two columns like location and message. But this is fine for now :)

Copy link
Contributor Author

@EverlastingBugstopper EverlastingBugstopper Dec 18, 2020

Choose a reason for hiding this comment

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

yeah, the way it prints it is as so:

 ERROR [films] Person -> A @key directive specifies a field which is not found in this service. Add a field to this type with @external.
 ERROR [films] Person -> A @key selects iddd, but Person.iddd could not be found
 ERROR [films] Person -> extends from people but specifies an invalid @key directive. Valid @key directives are specified by the originating type. Available @key directives for this type are:
        @key(fields: "id")
Error: Encountered 3 composition errors while composing the subgraph

i think if we wanted to print it like a table we'd want to have that data returned in separate fields by the API rather than trying to parse out these strings.

src/command/subgraph/mod.rs Outdated Show resolved Hide resolved
@EverlastingBugstopper EverlastingBugstopper merged commit af28989 into main Jan 11, 2021
@EverlastingBugstopper EverlastingBugstopper deleted the avery/check-please branch January 11, 2021 20:19
StephenBarlow pushed a commit that referenced this pull request Jan 11, 2021
lrlna pushed a commit that referenced this pull request Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

new commands: schema check + service check
3 participants