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

upload index.json from artifacts upload #1622

Merged
merged 3 commits into from
Feb 17, 2022

Conversation

tianleh
Copy link
Member

@tianleh tianleh commented Feb 15, 2022

Description

As #1569 grows, extract the logic to upload index.json to current PR.

Issues Resolved

See related PR above.

Test

  1. update unit tests
  2. use playground Jenkins for testing.

The build link
http://jenki-jenki-fpgmrv2ryxko-1366042710.us-east-1.elb.amazonaws.com/job/Playground/job/tianleh-distribution-url-latest/14/

Verified that the latest build number is correct.
https://ci.opensearch.org/ci/dbc/Playground/tianleh-distribution-url-latest/2.0.0/index.json
Screen Shot 2022-02-15 at 2 09 24 AM

Check List

  • Commits are signed per the DCO using --signoff

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.

@tianleh tianleh requested a review from a team as a code owner February 15, 2022 10:09
@tianleh
Copy link
Member Author

tianleh commented Feb 15, 2022

Want to call out one build during my testing.
http://jenki-jenki-fpgmrv2ryxko-1366042710.us-east-1.elb.amazonaws.com/job/Playground/job/tianleh-distribution-url-latest/13/

This build (build 13) failed but the index.json file is updated. When I rebuild the job, it succeeded (build 14). Thus looks like a transient failure in the build job.

I will check more to see if this is a concern.

cc @dblock @seraphjiang

@peternied
Copy link
Member

This build (build 13) failed but the index.json file is updated. When I rebuild the job, it succeeded (build 14). Thus looks like a transient failure in the build job.

The failure was from the snapshot build, not from the x64/arm64. A 'transient' failure would be because of a network hiccup, the as the builds output files are expected to be fully deterministic, and the error was a file was not found. Is it possible there was another change on your branch that altered the behavior.

Uploading file:/.../tianleh-distribution-url-latest/builds/opensearch/dist/opensearch-min-2.0.0-SNAPSHOT-linux-arm64-latest.tar.gz to s3://.../snapshots/core/opensearch/2.0.0-SNAPSHOT/opensearch-min-2.0.0-SNAPSHOT-linux-arm64-latest.tar.gz 
Upload failed due to missing source file

I would recommend working with @abhinavGupta16 on a test for the opensearch distribution build to make sure the behavior you've added isn't impacting snapshot builds unexpectedly

@peternied peternied removed their request for review February 15, 2022 17:34
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

You should add a Groovy test for getIndexFileRoot, otherwise LGTM.

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2022

Codecov Report

Merging #1622 (90d892c) into main (bddbc06) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1622   +/-   ##
=========================================
  Coverage     94.67%   94.67%           
- Complexity       13       14    +1     
=========================================
  Files           163      163           
  Lines          3437     3438    +1     
  Branches         21       21           
=========================================
+ Hits           3254     3255    +1     
  Misses          180      180           
  Partials          3        3           
Impacted Files Coverage Δ
tests/jenkins/jobs/BuildManifest_Jenkinsfile 100.00% <ø> (ø)
src/jenkins/BuildManifest.groovy 91.66% <100.00%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bddbc06...90d892c. Read the comment docs.

@tianleh
Copy link
Member Author

tianleh commented Feb 16, 2022

This build (build 13) failed but the index.json file is updated. When I rebuild the job, it succeeded (build 14). Thus looks like a transient failure in the build job.

The failure was from the snapshot build, not from the x64/arm64. A 'transient' failure would be because of a network hiccup, the as the builds output files are expected to be fully deterministic, and the error was a file was not found. Is it possible there was another change on your branch that altered the behavior.


Uploading file:/.../tianleh-distribution-url-latest/builds/opensearch/dist/opensearch-min-2.0.0-SNAPSHOT-linux-arm64-latest.tar.gz to s3://.../snapshots/core/opensearch/2.0.0-SNAPSHOT/opensearch-min-2.0.0-SNAPSHOT-linux-arm64-latest.tar.gz 

Upload failed due to missing source file

I would recommend working with @abhinavGupta16 on a test for the opensearch distribution build to make sure the behavior you've added isn't impacting snapshot builds unexpectedly

The build is using the branch of this PR and thus the touched code is all in the PR.

Copy link
Member

@gaiksaya gaiksaya left a comment

Choose a reason for hiding this comment

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

LGTM!

@tianleh
Copy link
Member Author

tianleh commented Feb 17, 2022

Looks like I do not have write access to merge approved pull requests. Could you please help merge it or grant me such write permission? @dblock @gaiksaya

Screen Shot 2022-02-16 at 4 32 06 PM

@gaiksaya gaiksaya merged commit 1b18900 into opensearch-project:main Feb 17, 2022
@tianleh
Copy link
Member Author

tianleh commented Mar 2, 2022

Link to main issue #1492

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.

5 participants