Skip to content

fix: removes unwrap from is_federated query#549

Merged
EverlastingBugstopper merged 1 commit intomainfrom
avery/really-fix-panic
May 19, 2021
Merged

fix: removes unwrap from is_federated query#549
EverlastingBugstopper merged 1 commit intomainfrom
avery/really-fix-panic

Conversation

@EverlastingBugstopper
Copy link
Copy Markdown
Contributor

@EverlastingBugstopper EverlastingBugstopper commented May 17, 2021

Fixes #548 by removing the .unwrap call in the is_federated run that occurs if anything goes wrong with the query (for instance, mistyping your graph name).

This PR includes a few other refactors:

  • No longer run the is_federated check if the --convert flag is passed to subgraph publish, removing the extra network hop
  • Perform the is_federated query directly in the rover_client::query modules that need them, instead of in rover code.
    • This includes returning RoverClientError::ExpectedFederatedGraph directly from rover_client instead of constructing the error manually in rover
    • We still keep the suggestion for using --convert 😄
  • Replaced some verbose matches with a more ergonomic .ok_or

Also fixes #550

Comment thread crates/rover-client/src/query/graph/publish.rs
Comment thread src/command/subgraph/check.rs Outdated
Comment thread crates/rover-client/src/query/config/is_federated.rs Outdated
@EverlastingBugstopper EverlastingBugstopper merged commit a9693f8 into main May 19, 2021
@EverlastingBugstopper EverlastingBugstopper deleted the avery/really-fix-panic branch May 19, 2021 19:35
@EverlastingBugstopper EverlastingBugstopper modified the milestones: May 25, May 19 May 19, 2021
},
&client,
)?;
if is_federated {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this should be !is_federated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix 🩹 fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: crashed while attempting to upload with APOLLO_KEY set to some garbled value bug: crashed while publishing first federated service to graph

3 participants