Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Added URL for jb_scheduler-plugin_zip instead of local file path #82

Merged

Conversation

amirmuminovic
Copy link
Contributor

*Issue #79 *

Description of changes:
The build process failed on Windows because of a UNIX-specific path. During the build, there would be an attempt to download the file from the local file system which would result in an error, as shown in issue #79 .

The change included adding a remote URL from which the job-scheduler plugin would be downloaded instead of a local file reference.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

build.gradle Outdated
@@ -44,7 +44,7 @@ repositories {
ext {
opendistroVersion = '1.4.0'
isSnapshot = "true" == System.getProperty("build.snapshot", "true")
job_scheduler_plugin_zip = "file://${fileTree("src/test/resources/job-scheduler").getSingleFile().absolutePath}"
job_scheduler_plugin_zip = "https://github.com/opendistro-for-elasticsearch/job-scheduler/releases/download/v1.4.0.0/opendistro-job-scheduler-1.4.0.0.zip"
Copy link
Member

Choose a reason for hiding this comment

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

This adds a dependency on github when we build. Also, we need to upload a new zip to github in the future when we upgrade job-scheduler. I suggest detecting windows in gradle and use different paths instead. See https://stackoverflow.com/questions/11235614/how-to-detect-the-current-os-from-gradle/31443955

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kaituo I had a closer inspection of the issue. It was like @wnbts suggested, an error with the path on Windows. My initial solution was treating the symptom, not the root cause of the error.

I did some research and found the issue is that Linux requires two slashes when specifying the path whilst Windows requires three. I have made the adjustment and we should have no additional dependencies after this fix :D

Copy link
Member

Choose a reason for hiding this comment

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

Thank you very much for the change.

@wnbts wnbts merged commit 4e4b343 into opendistro-for-elasticsearch:development Apr 13, 2020
kaituo pushed a commit to kaituo/anomaly-detection that referenced this pull request Apr 15, 2020
…ndistro-for-elasticsearch#82)

* Added URL for jb_scheduler-plugin_zip instead of local file path

* Fixed windows path by adding additional /
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants