-
Notifications
You must be signed in to change notification settings - Fork 25
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
Create publishToNpm library #21
Conversation
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
@@ -40,7 +40,6 @@ void call(Map args = [:], Closure body) { | |||
always { | |||
script { | |||
postCleanup() | |||
sh 'docker image prune -f --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.
Why remove this?
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.
Caused an error looking for docker command on agent node.
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.
Will this affect any other jenkinsfile that calls this?
Ex: We want to do the cleanup for docker build.
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.
No, this file is not used anywhere today.
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.
shouldn't agent node supposed to have docker installed? @gaiksaya
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.
@rishabh6788 this file is using docker container running on a jenkins linux agent.
The agent can have docker but the container doesnt have to.
Except docker builder.
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.
understood, thank you!
checkout([$class: 'GitSCM', branches: [[name: "${args.tag}" ]], userRemoteConfigs: [[url: "${args.repository}" ]]]) | ||
|
||
withCredentials([string(credentialsId: 'publish-to-npm-token', variable: 'NPM_TOKEN')]){ | ||
sh """npm set registry "https://registry.npmjs.org"; npm set //registry.npmjs.org/:_authToken ${NPM_TOKEN}; npm publish --dry-run && npm publish --access public""" |
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.
Prefer writing this in multiple lines for easier visibility.
sh """
<code>
"""
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.
Makes its difficult to test things due to whitespace. Hence used the above approach. Similar to https://github.com/opensearch-project/opensearch-build-libraries/blob/main/vars/copyContainer.groovy#L35
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.
Huh, I do find it not ideal for reading, what would happen if the code block is too long to put into one line?
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.
This was not long enough that's why used this way. If its very long we can use the approach you suggested or try to break it down.
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 think from my experience, we used to publish the client to staging NPM before final publish. Would that be performed on the Jenkinsfile level which calls this library?
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.
@zelinh We won't be dealing with staging. If component team wants to follow that they can do on their own. We will only be directly releasing on prod platform
checkout([$class: 'GitSCM', branches: [[name: "${args.tag}" ]], userRemoteConfigs: [[url: "${args.repository}" ]]]) | ||
|
||
withCredentials([string(credentialsId: 'publish-to-npm-token', variable: 'NPM_TOKEN')]){ | ||
sh """npm set registry "https://registry.npmjs.org"; npm set //registry.npmjs.org/:_authToken ${NPM_TOKEN}; npm publish --dry-run && npm publish --access public""" |
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 think from my experience, we used to publish the client to staging NPM before final publish. Would that be performed on the Jenkinsfile level which calls this library?
checkout([$class: 'GitSCM', branches: [[name: "${args.tag}" ]], userRemoteConfigs: [[url: "${args.repository}" ]]]) | ||
|
||
withCredentials([string(credentialsId: 'publish-to-npm-token', variable: 'NPM_TOKEN')]){ | ||
sh """npm set registry "https://registry.npmjs.org"; npm set //registry.npmjs.org/:_authToken ${NPM_TOKEN}; npm publish --dry-run && npm publish --access public""" |
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.
Maybe we could add one another command npm logout
at the beginning to make sure we don't accidentally publish to somewhere else or this could be replaced with new login?
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.
We are using a token and not logging into the account so no need to log out. The scope of scope is within the withCredentials
.
@zelinh We won't be dealing with staging. If component team wants to follow that they can do on their own. We will only be directly releasing on prod platform |
* Add npm publishing lib Signed-off-by: Sayali Gaikawad <[email protected]>
* Added precision for codecov (#17) * Added precision for codecov Signed-off-by: Owais Kazi <[email protected]> * Add standard release pipeline library (#11) * Add standard release pipeline library Signed-off-by: Sayali Gaikawad <[email protected]> * Create publishToNpm library (#21) * Add npm publishing lib Signed-off-by: Sayali Gaikawad <[email protected]> * Add standard release pipeline library with generic trigger (#22) Signed-off-by: Sayali Gaikawad <[email protected]> * Add release workflow and readme (#23) * Add release.yml Signed-off-by: Sayali Gaikawad <[email protected]> * Fix releasing.md (#25) Signed-off-by: Sayali Gaikawad <[email protected]> Signed-off-by: Sayali Gaikawad <[email protected]> * Fix credential type for github bot (#26) * Fix credential type for github bot Signed-off-by: Sayali Gaikawad <[email protected]> * Add untriaged label to new github issues (#27) Signed-off-by: Rishabh Singh <[email protected]> Signed-off-by: Rishabh Singh <[email protected]> * Remove docker check for windows gradle check (#28) * Remove docker check for windows gradle check Signed-off-by: Peter Zhu <[email protected]> * add test results Signed-off-by: Peter Zhu <[email protected]> Signed-off-by: Peter Zhu <[email protected]> * Remove docker check for windows gradle check (#28) * Remove docker check for windows gradle check Signed-off-by: Peter Zhu <[email protected]> * add test results Signed-off-by: Peter Zhu <[email protected]> Signed-off-by: Peter Zhu <[email protected]> * upgrade to 1.1.1 with changes in #28 Signed-off-by: Peter Zhu <[email protected]> * Test results Signed-off-by: Peter Zhu <[email protected]> Signed-off-by: Owais Kazi <[email protected]> Signed-off-by: Sayali Gaikawad <[email protected]> Signed-off-by: Rishabh Singh <[email protected]> Signed-off-by: Peter Zhu <[email protected]> Co-authored-by: Owais Kazi <[email protected]> Co-authored-by: Sayali Gaikawad <[email protected]> Co-authored-by: Rishabh Singh <[email protected]>
Description
Adds publishToNpm library that can be used to publish packages to NPM under
@opensearch-project
name space.The library takes 2 inputs which is Github repository and the tag
Issues Resolved
resolves opensearch-project/opensearch-build#2637
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.