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

Fix/improve watcher test scripts: YAML support, metadata-git-commit, better error handling, --cacert, multiple time fields #239

Merged
merged 31 commits into from
Jul 23, 2021

Conversation

ypid-geberit
Copy link
Contributor

Using YAML comes in handy for bigger watches 😉

Needed for: elastic/elasticsearch#30112 (comment)

> I already have a workaround in place for this which consists of automatically deploying as many watches as I need to send different mails. Those watches are derived from my watch definition. For this, I extended [run_test.py](https://github.com/elastic/examples/blob/master/Alerting/Sample%20Watches/run_test.py) to inject Python code after the watch definition is read. Not ideal, but it is maintainable.
Needed for: elastic/elasticsearch#30112 (comment)

> I already have a workaround in place for this which consists of automatically deploying as many watches as I need to send different mails. Those watches are derived from my watch definition. For this, I extended [run_test.py](https://github.com/elastic/examples/blob/master/Alerting/Sample%20Watches/run_test.py) to inject Python code after the watch definition is read. Not ideal, but it is maintainable.
Needed for: elastic/elasticsearch#30112 (comment)

> I already have a workaround in place for this which consists of automatically deploying as many watches as I need to send different mails. Those watches are derived from my watch definition. For this, I extended [run_test.py](https://github.com/elastic/examples/blob/master/Alerting/Sample%20Watches/run_test.py) to inject Python code after the watch definition is read. Not ideal, but it is maintainable.
Useful when you have two time fields that in reality should be very
close so in testing it is enough to set them to the same value.
@ypid-geberit ypid-geberit force-pushed the improve/watcher-test-scripts branch from 8699e59 to b48043c Compare June 25, 2020 16:54
@ypid-geberit ypid-geberit changed the title Improve/watcher test scripts Fix/improve watcher test scripts: ES 7, YAML support, Py3, metadata-git-commit Jun 25, 2020
@ypid-geberit
Copy link
Contributor Author

As far as I can tell, the version of run_test.py in master is incompatible with ES 7.x. Any plans to merge those improvements?

@DanRoscigno
Copy link
Contributor

@ypid-geberit :
Hi Robin, if you can rebase this I will review the parts that I understand, and I will look for reviewers for the parts that I do not personally know about. Sorry for letting this sit for so long!

@ypid-geberit
Copy link
Contributor Author

ypid-geberit commented Sep 16, 2020

Done that. I went with merging the latest master into my branch which was way easier because here in this PR, I did some radical changes of run_test.py early on which git rebase cannot resolve automatically.

Note that we could drop --minify-scripts which was a workaround for ES before 6.6.0.

Edit: The force push you see below was only me cleaning up the git history by splitting up commits. One commit should only do one thing. I did not change any code other than fixing an issue in efad59e so the review by DanRoscigno should still be valid.

@ypid-geberit ypid-geberit force-pushed the improve/watcher-test-scripts branch from 5ad5e4e to 36c5cbe Compare September 16, 2020 17:00
Copy link
Contributor

@DanRoscigno DanRoscigno left a comment

Choose a reason for hiding this comment

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

run_all_tests.sh LGTM

@gingerwizard can you have a look at the diffs to run_all_tests.py

@legalastic can you comment on the copyright lines of run_all_tests.py:

SPDX-FileCopyrightText: 2017 Dale McDiarmid dalem@elastic.co

SPDX-FileCopyrightText: 2017-2020 Robin Schneider robin.schneider@geberit.com

SPDX-FileCopyrightText: 2020 Dan Roscigno dan@roscigno.com

SPDX-License-Identifier: Apache-2.0

run_test.sh LGTM

@ypid-geberit ypid-geberit changed the title Fix/improve watcher test scripts: ES 7, YAML support, Py3, metadata-git-commit Fix/improve watcher test scripts: ~~ES 7~~, YAML support, Py3, metadata-git-commit Sep 17, 2020
@ypid-geberit ypid-geberit changed the title Fix/improve watcher test scripts: ~~ES 7~~, YAML support, Py3, metadata-git-commit Fix/improve watcher test scripts: YAML support, metadata-git-commit, better error handling, --cacert, multiple time fields Sep 17, 2020
@ypid-geberit ypid-geberit force-pushed the improve/watcher-test-scripts branch from 36c5cbe to 97bfa9b Compare September 17, 2020 08:28
@ypid-geberit
Copy link
Contributor Author

Ping @DanRoscigno, @legalastic @gingerwizard

I still think this is a useful enhancement. What do you think?

@ypid-geberit
Copy link
Contributor Author

@DanRoscigno, @legalastic @gingerwizard: Any chance of getting this merged?

@DanRoscigno DanRoscigno merged commit dbc75a9 into elastic:master Jul 23, 2021
@DanRoscigno
Copy link
Contributor

Done, thanks @ypid-geberit

@ypid-geberit
Copy link
Contributor Author

So long and then so quickly merged. You are welcome, thanks for merging :)

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