-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
Ensure that schedule is a valid Cron expression #1104
Conversation
Docker image for this PR was built and is available on Docker Hub. You can pull it locally via the CLI: docker pull tomkerkhove/promitor-agent-scraper-ci:pr1104-linux Want to verify the new version? Run it locally: docker run -d -p 8999:80 --name promitor-agent-scraper-pr1104-linux \
--env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
--env-file C:/Promitor/az-mon-auth.creds \
--volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
--volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
tomkerkhove/promitor-agent-scraper-ci:pr1104-linux You can find a CI version of our Helm chart on hub.helm.sh |
Will this happen during startup validation or before? Just asking to see if we should merge them or not to provide a consistent experience. For the rest it looks great! |
It happens just after the config is deserialized and we check if there's any errors before proceeding. The error appears in the same place as the errors in the examples I've added to the PR about suggestions. |
Docker image for this PR was built and is available on Docker Hub. You can pull it locally via the CLI: docker pull tomkerkhove/promitor-agent-scraper-ci:pr1104-linux Want to verify the new version? Run it locally: docker run -d -p 8999:80 --name promitor-agent-scraper-pr1104-linux \
--env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
--env-file C:/Promitor/az-mon-auth.creds \
--volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
--volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
tomkerkhove/promitor-agent-scraper-ci:pr1104-linux You can find a CI version of our Helm chart on hub.helm.sh |
I've added the framework to perform additional validation on a field, and implemented a Cron expression validator for the scraping schedule. Each field can have a collection of zero or more validators that implement `IFieldValidator`, and the validator uses the error reporter to report any validation errors. Fixes tomkerkhove#1103
888992e
to
1f87405
Compare
Docker image for this PR was built and is available on Docker Hub. You can pull it locally via the CLI: docker pull tomkerkhove/promitor-agent-scraper-ci:pr1104-linux Want to verify the new version? Run it locally: docker run -d -p 8999:80 --name promitor-agent-scraper-pr1104-linux \
--env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
--env-file C:/Promitor/az-mon-auth.creds \
--volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
--volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
tomkerkhove/promitor-agent-scraper-ci:pr1104-linux You can find a CI version of our Helm chart on hub.helm.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks man!
Docker image for this PR was built and is available on Docker Hub. You can pull it locally via the CLI: docker pull tomkerkhove/promitor-agent-scraper-ci:pr1104-linux Want to verify the new version? Run it locally: docker run -d -p 8999:80 --name promitor-agent-scraper-pr1104-linux \
--env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
--env-file C:/Promitor/az-mon-auth.creds \
--volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
--volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
tomkerkhove/promitor-agent-scraper-ci:pr1104-linux You can find a CI version of our Helm chart on hub.helm.sh |
I've added the framework to perform additional validation on a field, and implemented a Cron expression validator for the scraping schedule.
Each field can have a collection of zero or more validators that implement
IFieldValidator
, and the validator uses the error reporter to report any validation errors.Fixes #1103
If we use the following example config:
We'll now get the following error message: