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

Add sample Hello World Scheduled Job #692

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Apr 22, 2023

Description

Adds sample of a scheduled job to the HelloWorld sample Extension. The job is simple and prints Hello, World! every minute by default and the API has no configurability at the moment and cannot be de-scheduled. I plan to follow-up this PR with another that introduces an API to de-schedule the job and try to make this API more interesting.

You can call on the API with the following route:

curl -XPUT http://localhost:9200/_extensions/_hw/schedule/hello

In order to test this you need to run an OpenSearch node with the Job Scheduler plugin installed.

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.

Signed-off-by: Craig Perkins <[email protected]>
@dbwiddis
Copy link
Member

Looks like the gradle check is failing on the Job Scheduler imports. Do they need to be added to build.gradle?

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
@dbwiddis
Copy link
Member

Hey @cwperks I know I suggested you contribute this here, but now that I realize it requires a dependency on job scheduler I'm wondering if that's a good idea. We want to keep SDK as minimal as possible, and adding dependencies just for hello world doesn't really help there.

So I definitely think we need this functionality somewhere, but I think we need to take some steps to either separate hello world from the SDK, or start clean with new extension functionality outside the SDK. I'm sure in addition to this job scheduler functionality we'll need to add in security features as well. I'm not sure what that's going to look like yet.

@owaiskazi19 @saratvemulapalli @joshpalis @ryanbogan what are your thoughts here?

@cwperks
Copy link
Member Author

cwperks commented Apr 24, 2023

Hey @dbwiddis I agree that it doesn't make sense to add the dependencies only for the sample extension. If the sample extension was in another repo and it was only for the sample then that could make sense. Do you think it would be possible to include a separate build.gradle file for the sample extension that includes the dependencies?

@owaiskazi19
Copy link
Member

Hey @cwperks! Thanks for adding the job scheduler functionality to the hello world extension. I second to what @dbwiddis has pointed above to not pull job-scheduler dependency for a sample extension. I see 2 way to move forward here:

  1. Pull hello world extension to a separate repo. I don't see any problem in that. This will also help us to keep adding functionality like this one without worrying about impacting the SDK.
  2. Create a separate build.gradle file for hello world extension and keep it away from SDK's build.gradle.

I'm fine with both the approaches but more inclined towards the first one!

@dbwiddis
Copy link
Member

I see 2 way to move forward here:

  1. Pull hello world extension to a separate repo. I don't see any problem in that. This will also help us to keep adding functionality like this one without worrying about impacting the SDK.
  2. Create a separate build.gradle file for hello world extension and keep it away from SDK's build.gradle.

I'm fine with both the approaches but more inclined towards the first one!

Were it my private repo or a small org I control, I'd go with 1. But under the opensearch-project org, that just creates a whole new level of administrative handling, separate maintainer list, etc.

We're really doing somewhat the equivalent of the sample plugins which live underneath the OpenSearch project in their own modules. That's probably the way to go. But it also requires learning how to do gradle multimodule builds, which someone should do anyway. :)

One complicationg factor is that we do currently use helloworld tests to count toward our code coverage, so moving it elsewhere would require us to redo some tests to use an anonymous extension instead. Easily(?) done but a pain.

I propose we leave helloworld where it is, but intentionally keep it very simple. But I think we can really make our documentation awesome for future SDK users by adding this "add job scheduler functionality" to the whole "create your first extension" document, or a linked document for "next steps".

@cwperks
Copy link
Member Author

cwperks commented Apr 25, 2023

Closing this for now, I will spend some time looking at a separate build.gradle file but it may take some time to figure out how to get that to properly work. There should definitely be documentation added to CREATE_YOUR_FIRST_EXTENSION on how to create an extension that schedules jobs.

@cwperks cwperks closed this Apr 25, 2023
@cwperks cwperks reopened this May 1, 2023
@cwperks
Copy link
Member Author

cwperks commented May 1, 2023

Re-opening this PR, I made this into a multi-module/project gradle project with the sample extension as one of the modules. It now has a separate build.gradle file so I can introduce the job-scheduler dependencies without having the SDK depend on job-scheduler.

Signed-off-by: Craig Perkins <[email protected]>
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Merging #692 (69862c3) into main (eff7635) will decrease coverage by 3.35%.
The diff coverage is n/a.

❗ Current head 69862c3 differs from pull request most recent head 9f805c2. Consider uploading reports for the commit 9f805c2 to get more accurate results

@@             Coverage Diff              @@
##               main     #692      +/-   ##
============================================
- Coverage     43.57%   40.22%   -3.35%     
+ Complexity      314      269      -45     
============================================
  Files            69       61       -8     
  Lines          1983     1857     -126     
  Branches        141      133       -8     
============================================
- Hits            864      747     -117     
+ Misses         1101     1096       -5     
+ Partials         18       14       -4     

see 21 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants