Skip to content

Support validation parameters for service:check#953

Merged
trevor-scheer merged 3 commits intomasterfrom
trevor/service-check-moment
Feb 6, 2019
Merged

Support validation parameters for service:check#953
trevor-scheer merged 3 commits intomasterfrom
trevor/service-check-moment

Conversation

@trevor-scheer
Copy link
Copy Markdown
Contributor

@trevor-scheer trevor-scheer commented Jan 31, 2019

This PR adds functionality for 3 new flags in the service:check command:

  • validationPeriod - the duration (from now) to validate the service against
  • queryCountThreshold - minimum number of requests within the requested time window for a query to be considered
  • queryCountThresholdPercentage - number of requests within the requested time window for a query to be considered, relative to total request count.

Also included in this PR - better error messaging:

  • Reorders error throwing in EngineClient, so that errors reported from the service will be thrown
    with priority over the generic "noServiceError" when data isn't present.

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.

Comment thread packages/apollo/package.json Outdated
"heroku-cli-util": "^8.0.9",
"listr": "^0.14.1",
"lodash": "^4.17.10",
"moment": "^2.24.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use something lighter than moment like date-fns?

Copy link
Copy Markdown
Contributor Author

@trevor-scheer trevor-scheer Feb 1, 2019

Choose a reason for hiding this comment

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

Looked for anything to support durations with the ease-of-use + granularity that ISO 8601 gives us and no love, + discussed offline. Going to leave as is. If having moment gets too heavy, we can write a duration parser.

Comment thread packages/apollo/src/commands/service/check.ts Outdated
@trevor-scheer trevor-scheer force-pushed the trevor/service-check-moment branch from 885b61c to e1f4974 Compare February 5, 2019 21:20
* validationPeriod - the duration (from now) to validate the service against
* queryCountThreshold - minimum number of requests within the requested time window for a query to be considered
* queryCountThresholdPercentage - number of requests within the requested time window for a query to be considered, relative to total request count.
Throw errors from the response if we have them, then throw the generic
error if no data is provided. Previously, the order of these were swapped,
meaning that if a specific error came back from the server, we would still
throw the generic error.
@trevor-scheer trevor-scheer force-pushed the trevor/service-check-moment branch from e1f4974 to 1399b04 Compare February 6, 2019 16:07
@trevor-scheer trevor-scheer merged commit 1b53b1e into master Feb 6, 2019
@trevor-scheer trevor-scheer deleted the trevor/service-check-moment branch February 6, 2019 16:28
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.

2 participants