Alerting Plugins #93

arnavdas88 · 2021-11-20T13:20:28Z

Signed-off-by: arnavdas88 [email protected]

Description

These commits are regarding the issue #90 . This is just the initial commit with OpenSearch.plugins.alerting.*.

Issues Resolved

The issue of creating alerts from the python client.

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

stockholmux · 2021-12-03T16:20:45Z

Looks like lint is failing, can you take a look @arnavdas88?

arnavdas88 · 2021-12-03T16:22:04Z

@stockholmux Yup... Checking the code....

stockholmux · 2021-12-03T17:38:02Z

@arnavdas88 Can't tell you what's going on with some of these failures, but the DCO check failing is straight forward - I like this guide for fixing DCO issues.

arnavdas88 · 2021-12-03T17:42:29Z

Actually, 2 of the commits in between didn't take the '--signoff' switch for some reason... I am doing some rebase to fix the issue!! Sorry for the inconviniance

arnavdas88 · 2021-12-03T17:51:22Z

Now the previously unsigned commits are gone... so hopefully, it should work now!!!

stockholmux · 2021-12-06T20:04:12Z

@VijayanB is #97 related to the failures on this PR?

axeoman

Thanks a lot for your contribution. 👍
Overall looks good, there are, however, two issues that I would like to ask to change:

CI lint currently is failing because stubs are located in the incorrect path, e.g. out/opensearchpy/plugins/alerting.pyi instead of opensearchpy/plugins/alerting.pyi
It would be cool if you squash commits into one or prepare a meaningful list of commits in the PR. Of course, squash could be done by the person who will merge this, but it is always better to be done by the author.

P.S. Integration tests are failing because of #97 for sure.
edit: we probably will benefit from some regression tests checking that alerting plugin is called correctly, but this could be done separately.

opensearchpy/client/plugins.py

opensearchpy/plugins/alerting.py

VijayanB · 2021-12-15T03:00:24Z

out/opensearchpy/client/plugins.pyi

@@ -0,0 +1,7 @@
+from ..plugins.alerting import AlertingClient as AlertingClient


please add license header

Need license headers here.

opensearchpy/plugins/alerting.py

arnavdas88 · 2021-12-15T05:35:07Z

@VijayanB Give me a few minutes... i will apply the changes...

arnavdas88 · 2021-12-15T09:24:57Z

@VijayanB , I have squashed and pushed new commits with "license headers" and "removed warnings". Could you please check if the issues you pointed are solved or not.

axeoman · 2021-12-15T09:43:06Z

@arnavdas88 There are now 18 commits that are not good to push into main. Do you mind squashing everything (or splitting into a couple) and force pushing to a branch? If you want to fetch changes from main into your branch - better rebase on top of the master to avoid these "merge" commits.
Does it make sense to you? If not, we can just squash everything to one commit while merging anyway :)

arnavdas88 · 2021-12-15T11:22:08Z

@arnavdas88 There are now 18 commits that are not good to push into main. Do you mind squashing everything (or splitting into a couple) and force pushing to a branch? If you want to fetch changes from main into your branch - better rebase on top of the master to avoid these "merge" commits. Does it make sense to you? If not, we can just squash everything to one commit while merging anyway :)

Squashed into 1 single commit... Finally... 😆

Signed-off-by: Arnav Das <[email protected]>

axeoman · 2021-12-29T20:51:41Z

@dez @VijayanB do you have any complaints? Otherwise, I would merge it.

VijayanB · 2021-12-29T21:21:16Z

@dez @VijayanB do you have any complaints? Otherwise, I would merge it.

Thanks for checking. Let me take one more pass at review.

VijayanB · 2021-12-29T21:41:11Z

@arnavdas88 shall we add alerting to async as well?

harshavamsi · 2022-09-27T19:36:18Z

@arnavdas88 @VachaShah @dblock the only check failing is whitesource which is addressed here. The issue is not relevant here as we're building the right versions of the library where appropriate and is harmless against older versions that still use it.

Also, @arnavdas88 we should be incrementing the version to 2.1.0.

dblock

I am good with this! Any objections anyone else? Will wait a few days to hear back and/or merge.

harshavamsi · 2022-09-28T18:06:40Z

Opened #207 for docs and #206 to increment version.

VachaShah

Thank you @arnavdas88 for this feature!

VijayanB · 2022-09-28T23:39:14Z

test_opensearchpy/test_server_secured/__init__.py

+#
+#  Licensed to Elasticsearch B.V. under one or more contributor
+#  license agreements. See the NOTICE file distributed with
+#  this work for additional information regarding copyright
+#  ownership. Elasticsearch B.V. licenses this file to you under
+#  the Apache License, Version 2.0 (the "License"); you may
+#  not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+# 	http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing,
+#  software distributed under the License is distributed on an
+#  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+#  KIND, either express or implied.  See the License for the
+#  specific language governing permissions and limitations
+#  under the License.


Can you remove this and other places where is this introduced?

Right. If it's new code added after the fork we don't carry the ES license anymore.

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>

harshavamsi · 2022-09-29T17:35:25Z

Hi @arnavdas88 I forgot to remove some elastic licenses on files that have been added since the fork. Can you merge arnavdas88#2. Thanks.

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>

Remove elastic search licenses on new files

arnavdas88 · 2022-09-30T08:35:23Z

Hi @arnavdas88 I forgot to remove some elastic licenses on files that have been added since the fork. Can you merge arnavdas88#2. Thanks.

Thankyou @harshavamsi for fixing these issues in the code!!! I have merged the PR so you can run the workflow again...

VijayanB · 2022-09-30T17:21:02Z

opensearchpy/_async/http_aiohttp.py

@@ -97,7 +98,7 @@ def __init__(
        http_compress=None,
        opaque_id=None,
        loop=None,
-        **kwargs,
+        **kwargs


nit: Was this change introduced by styling tool? If not, can you revert here and other places? Thanks

VijayanB · 2022-09-30T17:21:54Z

opensearchpy/_async/plugins/alerting.py

+        """
+        Acknowledges an alert.
+
+        :arg monitor_id: The id of the monitor, the alert belongs to


nit: Doc doesn't reflect all parameters

VijayanB · 2022-09-30T17:23:18Z

opensearchpy/client/ingest.py

@@ -24,6 +24,7 @@
 #  specific language governing permissions and limitations
 #  under the License.

+


nit: if this is not introduced by styling tool, please revert this change.

dblock · 2022-09-30T18:59:19Z

@VijayanB Since the remaining items are nits, want to approve/merge? otherwise which ones are must haves?

dblock · 2022-10-03T18:45:31Z

Merged 🎆

stockholmux requested a review from rushiagr December 3, 2021 16:14

elfisher requested a review from VijayanB December 3, 2021 16:35

arnavdas88 force-pushed the main branch from 8bb35e6 to 1a684e3 Compare December 3, 2021 17:48

axeoman requested changes Dec 13, 2021

View reviewed changes

VijayanB reviewed Dec 15, 2021

View reviewed changes

opensearchpy/client/plugins.py Outdated Show resolved Hide resolved

VijayanB reviewed Dec 15, 2021

View reviewed changes

opensearchpy/client/plugins.py Show resolved Hide resolved

VijayanB reviewed Dec 15, 2021

View reviewed changes

opensearchpy/plugins/alerting.py Show resolved Hide resolved

VijayanB reviewed Dec 15, 2021

View reviewed changes

opensearchpy/plugins/alerting.py Outdated Show resolved Hide resolved

arnavdas88 force-pushed the main branch from 81cf2fc to 8ae5b3c Compare December 15, 2021 11:15

Alert Plugin

4b15a5e

Signed-off-by: Arnav Das <[email protected]>

arnavdas88 force-pushed the main branch from 8ae5b3c to 4b15a5e Compare December 15, 2021 12:22

arnavdas88 requested a review from axeoman December 16, 2021 11:59

arnavdas88 added 2 commits December 28, 2021 20:29

Merge branch 'opensearch-project:main' into main

d827041

lint file header license

c76236b

Signed-off-by: Arnav Das <[email protected]>

axeoman previously approved these changes Dec 29, 2021

View reviewed changes

VijayanB requested a review from Shivamdhar December 29, 2021 21:43

dblock previously approved these changes Sep 27, 2022

View reviewed changes

harshavamsi mentioned this pull request Sep 28, 2022

[FEATURE] Add documentation for plugin APIs in GETTING_STARTED and Sphinx docs #207

Closed

VachaShah previously approved these changes Sep 28, 2022

View reviewed changes

dblock requested a review from VijayanB September 28, 2022 22:04

VijayanB reviewed Sep 28, 2022

View reviewed changes

Remove elastic search licenses on new files

37dfd91

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>

harshavamsi and others added 2 commits September 29, 2022 17:40

Fix formatting

86c66cd

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>

Merge pull request #2 from harshavamsi/fix/license

9c14dae

Remove elastic search licenses on new files

arnavdas88 dismissed stale reviews from VachaShah and dblock via 9c14dae September 30, 2022 08:33

VijayanB reviewed Sep 30, 2022

View reviewed changes

VijayanB approved these changes Sep 30, 2022

View reviewed changes

VachaShah approved these changes Sep 30, 2022

View reviewed changes

VachaShah requested a review from dblock September 30, 2022 21:39

dblock merged commit 2d03dc7 into opensearch-project:main Oct 3, 2022

harshavamsi mentioned this pull request Oct 5, 2022

Adds alerting plugin related docs #213

Merged

wbeckler mentioned this pull request Nov 30, 2022

Release 2.0.1 has new features that would merit a minor version bump #248

Closed

Xtansia mentioned this pull request Mar 23, 2023

[PROPOSAL] Client Support For Extensions opensearch-project/opensearch-clients#55

Open

This was referenced Nov 10, 2023

[PROPOSAL] Plugins Python Client Interface #90

Closed

[FEATURE] Add high level DSL for k-nn plugin #300

Closed

[FEATURE] Add high level DSL for Cross Cluster Replication (CCR) #143

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alerting Plugins #93

Alerting Plugins #93

arnavdas88 commented Nov 20, 2021 •

edited

Loading

stockholmux commented Dec 3, 2021

arnavdas88 commented Dec 3, 2021

stockholmux commented Dec 3, 2021

arnavdas88 commented Dec 3, 2021

arnavdas88 commented Dec 3, 2021

stockholmux commented Dec 6, 2021

axeoman left a comment •

edited

Loading

VijayanB Dec 15, 2021

harshavamsi Sep 13, 2022

arnavdas88 commented Dec 15, 2021

arnavdas88 commented Dec 15, 2021

axeoman commented Dec 15, 2021

arnavdas88 commented Dec 15, 2021

axeoman commented Dec 29, 2021

VijayanB commented Dec 29, 2021

VijayanB commented Dec 29, 2021

harshavamsi commented Sep 27, 2022

dblock left a comment

harshavamsi commented Sep 28, 2022

VachaShah left a comment

VijayanB Sep 28, 2022

dblock Sep 29, 2022

harshavamsi commented Sep 29, 2022

arnavdas88 commented Sep 30, 2022

VijayanB Sep 30, 2022

VijayanB Sep 30, 2022

VijayanB Sep 30, 2022

dblock commented Sep 30, 2022

dblock commented Oct 3, 2022

		@@ -0,0 +1,7 @@
		from ..plugins.alerting import AlertingClient as AlertingClient

		@@ -24,6 +24,7 @@
		# specific language governing permissions and limitations
		# under the License.

Alerting Plugins #93

Alerting Plugins #93

Conversation

arnavdas88 commented Nov 20, 2021 • edited Loading

Description

Issues Resolved

Check List

stockholmux commented Dec 3, 2021

arnavdas88 commented Dec 3, 2021

stockholmux commented Dec 3, 2021

arnavdas88 commented Dec 3, 2021

arnavdas88 commented Dec 3, 2021

stockholmux commented Dec 6, 2021

axeoman left a comment • edited Loading

Choose a reason for hiding this comment

VijayanB Dec 15, 2021

Choose a reason for hiding this comment

harshavamsi Sep 13, 2022

Choose a reason for hiding this comment

arnavdas88 commented Dec 15, 2021

arnavdas88 commented Dec 15, 2021

axeoman commented Dec 15, 2021

arnavdas88 commented Dec 15, 2021

axeoman commented Dec 29, 2021

VijayanB commented Dec 29, 2021

VijayanB commented Dec 29, 2021

harshavamsi commented Sep 27, 2022

dblock left a comment

Choose a reason for hiding this comment

harshavamsi commented Sep 28, 2022

VachaShah left a comment

Choose a reason for hiding this comment

VijayanB Sep 28, 2022

Choose a reason for hiding this comment

dblock Sep 29, 2022

Choose a reason for hiding this comment

harshavamsi commented Sep 29, 2022

arnavdas88 commented Sep 30, 2022

VijayanB Sep 30, 2022

Choose a reason for hiding this comment

VijayanB Sep 30, 2022

Choose a reason for hiding this comment

VijayanB Sep 30, 2022

Choose a reason for hiding this comment

dblock commented Sep 30, 2022

dblock commented Oct 3, 2022

arnavdas88 commented Nov 20, 2021 •

edited

Loading

axeoman left a comment •

edited

Loading