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: add elasticsearch monitor (index and query) #3322

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

mhkarimi1383
Copy link
Contributor

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Adding Elasticsearch monitors to index data and query data

Type of change

Please delete any options that are not relevant.

  • User interface (UI)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.

mhkarimi1383 and others added 6 commits June 26, 2023 12:33
Signed-off-by: Muhammed Hussein Karimi <[email protected]>
* Add nostr DM notification provider

* require crypto for node 18 compatibility

* remove whitespace

Co-authored-by: Frank Elsinga <[email protected]>

* move closer to where it is used

* simplify success or failure logic

* don't clobber the non-alert msg

* Update server/notification-providers/nostr.js

Co-authored-by: Frank Elsinga <[email protected]>

* polyfills required for node <= 18

* resolve linter warnings

* missing comma

---------

Co-authored-by: Frank Elsinga <[email protected]>
Signed-off-by: Muhammed Hussein Karimi <[email protected]>
Signed-off-by: Muhammed Hussein Karimi <[email protected]>
Signed-off-by: Muhammed Hussein Karimi <[email protected]>
@louislam

This comment was marked as resolved.

@mhkarimi1383

This comment was marked as resolved.

@chakflying chakflying added the area:monitor Everything related to monitors label Dec 2, 2023
@919927181

This comment was marked as spam.

@CommanderStorm

This comment was marked as resolved.

@CommanderStorm CommanderStorm changed the title ✨ feat: add elasticsearch monitor (index and query) feat: add elasticsearch monitor (index and query) Apr 27, 2024
@CommanderStorm CommanderStorm added the question Further information is requested label Apr 27, 2024
@CommanderStorm

This comment was marked as resolved.

@mhkarimi1383
Copy link
Contributor Author

@CommanderStorm will do that
I have stopped because this project was in refactor state and I thought that monitor type section of project will change

@mhkarimi1383
Copy link
Contributor Author

mhkarimi1383 commented Apr 29, 2024

@mhkarimi1383 you can drop b8bcb6f from this PR via the following commands

I have done these steps

@mhkarimi1383
Copy link
Contributor Author

mhkarimi1383 commented Apr 29, 2024

you have disabled maintainer write access

Also edit is allowed by maintainers

@CommanderStorm

This comment was marked as resolved.

@CommanderStorm
Copy link
Collaborator

I have stopped because this project was in refactor state and I thought that monitor type section of project will

True.
Since then we have documented this part (how to add new monitors) in our contribution guide https://github.com/louislam/uptime-kuma/blob/19e8c75c3b7fa8e1e9d6675372785dc37a8ef04e/CONTRIBUTING.md#:~:text=new%20monitoring%20types
Older monitors still need to be refactored for the most part, but that should not be a merge blocker.

@mhkarimi1383
Copy link
Contributor Author

@CommanderStorm
Will do it
Finally, I see devcontainers here :)

@mhkarimi1383
Copy link
Contributor Author

@CommanderStorm
Branch updated

@CommanderStorm CommanderStorm added the pr:needs review this PR needs a review by maintainers or other community members label May 19, 2024
Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

Since then we have documented this part (how to add new monitors) in our contribution guide.
Older monitors still need to be refactored for the most part, but that should not be a merge blocker.

Okay.. what I meant by my above comment was that this PR is missing the Frontend and the backend partially.

=> please include:

  • the Frontend (see contribution guide what files need to be changed)
  • the Monitor (see contribution guide what files need to be changed)
  • how to setup elasicsearch in the simplest way ("how to test the PR")

Please also make sure that you have filled out the Checklist from the PR-description (f.ex. the linters)

@CommanderStorm CommanderStorm added pr:please address review comments this PR needs a bit more work to be mergable and removed pr:needs review this PR needs a review by maintainers or other community members labels May 19, 2024
@github-actions github-actions bot added the pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again label Jun 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors needs:resolve-merge-conflict pr:please address review comments this PR needs a bit more work to be mergable pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants