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: makes subgraph routing-url optional #484

Merged
merged 6 commits into from
May 3, 2021

Conversation

EverlastingBugstopper
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper commented Apr 28, 2021

fixes #169 it really is this simple!

  • If --routing-url is not passed and there is already a routing url for a managed service, the API should leave it untouched
  • If --routing-url is not passed and this is the first time they're pushing a service, we should error and tell them they need to pass the --routing-url flag
    • the error message could be a bit better here, currently it's error: URL must be provided when upserting a new service. something better might be error: You must provide a --routing-url when publishing a subgraph for the first time
  • If --routing-url is passed and it's the first time pushing a service, it should push the service and exit successfully.

@EverlastingBugstopper EverlastingBugstopper added the feature 🎉 new commands, flags, functionality, and improved error messages label Apr 28, 2021
@EverlastingBugstopper EverlastingBugstopper added this to the May 11 - GA milestone Apr 28, 2021
Copy link
Contributor

@JakeDawkins JakeDawkins left a comment

Choose a reason for hiding this comment

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

😂 you really do love to see it

Copy link
Member

@lrlna lrlna left a comment

Choose a reason for hiding this comment

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

I am wondering if we should include notes in our docs about these routing-url specifications. What do you all think?

@EverlastingBugstopper
Copy link
Contributor Author

EverlastingBugstopper commented May 3, 2021

Probably not a bad idea!

TODO:

  • add notes about upserting routing url in help-text and docs
  • add changelog entry

@EverlastingBugstopper EverlastingBugstopper merged commit f4e536e into main May 3, 2021
@EverlastingBugstopper EverlastingBugstopper deleted the avery/routing-url branch May 3, 2021 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎉 new commands, flags, functionality, and improved error messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvements on routing url option
4 participants