-
Notifications
You must be signed in to change notification settings - Fork 73
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
Artifacts path update at right location #376
Conversation
Signed-off-by: Varun Jain <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job figuring this out @vibrantvarun! I have checked out this branch and verified the changes locally.
build.gradle
Outdated
} | ||
repositories { | ||
maven { | ||
name = "Snapshots" // optional target repository name | ||
url = "https://aws.oss.sonatype.org/content/repositories/snapshots" | ||
url = "https://aws.oss.sonatype.org/content/repositories/snapshots/snapshots" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this line changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted it. Changed by mistake. Thanks for calling it out
developer.appendNode('url', 'https://github.com/opensearch-project/job-scheduler') | ||
} | ||
} | ||
// all { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the commented out lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took care off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still seeing commented out lines in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 lm still seeing commented code.
build.gradle
Outdated
@@ -136,11 +136,31 @@ publishing { | |||
groupId = "org.opensearch.plugin" | |||
} | |||
} | |||
mavenJava(MavenPublication){ publication -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: with the other publication called pluginZip
, do you think we should call this pluginJar
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Signed-off-by: Varun Jain <[email protected]>
Signed-off-by: Varun Jain <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me. Thanks @vibrantvarun .
Could you take a look at the CI failures?
@prudhvigodithi @gaiksaya how do we fix this for 2.7 published release after this change is merged?
developer.appendNode('url', 'https://github.com/opensearch-project/job-scheduler') | ||
} | ||
} | ||
// all { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 lm still seeing commented code.
Hi @vibrantvarun, Can you check if it is publishing as expected on maven local too? Details
Which are then downloaded from S3 and published using maven cli. Workflow: https://github.com/opensearch-project/opensearch-build/blob/main/jenkins/opensearch-maven-release/publish-to-maven.jenkinsfile#L39-L44 @saratvemulapalli @prudhvigodithi I believe we might need to document the process step by step of manual checking out the job scheduler repo at tag 2.7.0.0, cherry-pick this commit, get maven credentials and only run publish task for this URL: https://aws.oss.sonatype.org/content/repositories/releases/org/opensearch/opensearch-job-scheduler/ |
Description
The PR fixes the artifact publishing issue with job scheduler. I have added a new publication called mavenJava in the build .gradle which does packaging in Jar to the right publishing path.
Steps I followed to test:
Please find the test output below.
Issues Resolved
#374
Check List
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.