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.

Sign up for GitHub

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

Jump to bottom

Fix nightly upload to GH release logic. #29629

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Fix nightly upload to GH release logic. #29629

merged 1 commit into from
Apr 13, 2023

Conversation

mukilan
Copy link
Member

@mukilan mukilan commented Apr 13, 2023 •
edited

The boto3 S3 client automatically closes the given fileobj after the transfer is complete. This prevents us from reusing the package_hash_fileobj between s3 and github upload methods. This is causing the upload to github to fail with:

ValueError: I/O operation on closed file.

  File "/home/runner/work/servo/servo/python/servo/package_commands.py", line 792, in upload_nightly
    upload_to_github_release(platform, package, package_hash_fileobj)
  File "/home/runner/work/servo/servo/python/servo/package_commands.py", line 635, in upload_to_github_release
    package_hash_fileobj.getbuffer().nbytes,

This PR changes fixes the issue by creating fresh instances of io.BytesIO within the two upload_to_* methods.

I've triggered a manual nightly job based on this branch. This PR can be kept open until the build completes, in case other issues are surfaced.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because they fix an error in nightly CI Job

@mukilan
Fix nightly upload to GH release logic.
76a5a10 
The boto3 S3 client automatically closes the given
fileobj after the transfer is complete. This prevents
us from reusing the package_hash_fileobj between
s3 and github upload methods.

This PR changes fixes the issue by creating fresh
instances of io.BytesIO within the upload_to_*
methods.

Signed-off-by: Mukilan Thiyagarajan <me@mukilan.in>
@mukilan
Copy link
Member Author

mukilan commented Apr 13, 2023

@bors-servo r+

Thanks for the review! The test build has passed, so I'm merging this PR

@bors-servo
Copy link
Contributor

📌 Commit 76a5a10 has been approved by mukilan

@bors-servo
Copy link
Contributor

⌛ Testing commit 76a5a10 with merge 45000be...

@github-actions
Copy link

Results from try job (#4688410512):

Flaky unexpected result (20)
  • OK /_mozilla/css/stylesheet_media_queries.html (#17159)
    • FAIL [expected PASS] subtest: Media queries within stylesheets
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-with-non-reserved-words.html (#16216)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/CSS2/floats/hit-test-floats-005.html (#25804)
    • FAIL [expected PASS] subtest: Miss clipped float
  • OK /css/CSS2/linebox/inline-negative-margin-001.html (#23862)
    • FAIL [expected PASS] subtest: [data-expected-height] 1
    • FAIL [expected PASS] subtest: [data-expected-height] 2
    • PASS [expected FAIL] subtest: [data-expected-height] 4
  • OK /css/CSS2/normal-flow/block-in-inline-hittest-float-002.html (#29051)
    • FAIL [expected PASS] subtest: block-in-inline-hittest-float-002
  • OK /css/css-flexbox/hittest-before-pseudo.html (#26521)
    • PASS [expected FAIL] subtest: Hit-testing text within a pseudo-element flex-item should return the flexbox as the hittest result.
  • OK /css/css-transitions/properties-value-inherit-002.html (#21486)
    • FAIL [expected PASS] subtest: margin-right length(cm) / values
    • FAIL [expected PASS] subtest: margin-right length(in) / values
    • FAIL [expected PASS] subtest: margin-top length(pt) / values
    • FAIL [expected PASS] subtest: margin-top length(pc) / values
    • FAIL [expected PASS] subtest: margin-top length(px) / values
    • FAIL [expected PASS] subtest: margin-top length(em) / values
    • FAIL [expected PASS] subtest: margin-top length(ex) / values
    • FAIL [expected PASS] subtest: margin-top length(mm) / values
    • FAIL [expected PASS] subtest: margin-top length(cm) / values
    • FAIL [expected PASS] subtest: margin-top length(in) / values
    • And 90 more unexpected results...
  • OK /css/cssom-view/CaretPosition-001.html (#21338)
    • PASS [expected FAIL] subtest: Element at (400, 100)
  • OK /css/cssom-view/elementsFromPoint-invalid-cases.html (#21338)
    • PASS [expected FAIL] subtest: The root element is the last element returned for otherwise empty queries within the viewport
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
    • PASS [expected FAIL] subtest: load event does not fire on window.open('about:blank#foo')
  • TIMEOUT /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/window-open-204.html (#29241)
    • FAIL [expected TIMEOUT] subtest: location.href
    • TIMEOUT [expected NOTRUN] subtest: location.assign
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-cross-origin.sub.window.html (#29056)
    • FAIL [expected PASS] subtest: Cross-origin navigation started from unload handler must be ignored
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
    • FAIL [expected NOTRUN] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: 0x00 in name (normal form)
  • OK [expected ERROR] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • TIMEOUT [expected OK] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry-different-function-realm.html (#25805)
    • TIMEOUT [expected FAIL] subtest: Fulfillment handler on pending-then-fulfilled promise
    • TIMEOUT [expected FAIL] subtest: Rejection handler on pending-then-rejected promise
  • TIMEOUT [expected CRASH] /webmessaging/broadcastchannel/cross-partition.https.tentative.html (#29058)
  • OK /workers/dedicated-worker-from-blob-url.window.html (#22286)
    • PASS [expected FAIL] subtest: Creating a dedicated worker from a blob URL works immediately before revoking.
Stable unexpected results that are known to be intermittent (32)
  • TIMEOUT [expected OK] /FileAPI/url/url-charset.window.html (#26997)
    • TIMEOUT [expected PASS] subtest: Blob charset should override any auto-detected charset.
  • OK /_mozilla/mozilla/scrollBy.html (#21321)
    • FAIL [expected PASS] subtest: Ensure that the window.scrollBy function affects scroll position as expected
  • ERROR /_webgl/conformance/textures/misc/origin-clean-conformance-offscreencanvas.html (#24038)
    • PASS [expected FAIL] subtest: WebGL test #0: img was loaded
  • ERROR /_webgl/conformance2/textures/misc/origin-clean-conformance-offscreencanvas.html (#29589)
    • PASS [expected FAIL] subtest: WebGL test #0: img was loaded
  • OK /css/CSS2/floats/hit-test-floats-001.html (#23693)
    • FAIL [expected PASS] subtest: hit-test-floats-001
  • OK /css/CSS2/floats/hit-test-floats-002.html (#25733)
    • PASS [expected FAIL] subtest: Hit test float
  • OK /css/CSS2/normal-flow/block-in-inline-hittest-002.html (#29057)
    • PASS [expected FAIL] subtest: elementFromPoint
  • OK /css/CSS2/normal-flow/block-in-inline-hittest-relpos-zindex.html (#29052)
    • PASS [expected FAIL] subtest: block-in-inline-hittest-relpos-zindex
  • OK /css/CSS2/normal-flow/hit-test-anonymous-block.html (#25807)
    • PASS [expected FAIL] subtest: Hit test beside line of text inside anonymous block
  • OK /css/css-flexbox/hittest-anonymous-box.html (#27091)
    • PASS [expected FAIL] subtest: Hit-testing within an anonymous flex-item should return the flexbox as the hittest result.
  • OK /css/css-fonts/variations/font-weight-matching.html (#20686)
    • FAIL [expected PASS] subtest: Test @font-face matching for weight 400
    • FAIL [expected PASS] subtest: Test @font-face matching for weight 420
  • OK /css/css-transforms/inline-with-filter-and-hidden-backface.html (#29547)
    • PASS [expected FAIL] subtest: Hit test
  • OK /css/cssom-view/elementFromPoint-float-in-table.html (#27002)
    • PASS [expected FAIL] subtest: float-in-div
  • OK /css/cssom-view/elementFromPoint-list-001.html (#23915)
    • PASS [expected FAIL] subtest: <li>Inside 2</li>
    • PASS [expected FAIL] subtest: <li>Inside 3</li>
    • PASS [expected FAIL] subtest: <li>Image Inside 1</li>
    • PASS [expected FAIL] subtest: <li>Image Inside 2</li>
  • OK /css/cssom-view/elementFromPoint-visibility-hidden-resizer.html (#28695)
    • PASS [expected FAIL] subtest: elementFromPoint on resizer area of an element with visibility:hidden
  • OK /css/cssom-view/elementFromPosition.html (#21295)
    • FAIL [expected PASS] subtest: test the top of layer
  • OK /css/cssom-view/scroll-behavior-smooth-navigation.html (#29564)
    • FAIL [expected PASS] subtest: Instant scrolling while doing history navigation.
    • FAIL [expected PASS] subtest: Smooth scrolling while doing history navigation.
  • TIMEOUT [expected OK] /fetch/api/basic/keepalive.any.html (#29536)
  • OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
    • PASS [expected FAIL] subtest: Check execution order on load handler
    • PASS [expected FAIL] subtest: Check execution order from nested timeout
  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/failure-check-sequence.https.html (#28207)
    • FAIL [expected TIMEOUT] subtest: CSP check precedes X-Frame-Options check
    • FAIL [expected NOTRUN] subtest: CSP check precedes COEP check - CSP header first
    • FAIL [expected NOTRUN] subtest: CSP check precedes COEP check - COEP header first
    • FAIL [expected NOTRUN] subtest: COEP check precedes X-Frame-Options check
  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
    • FAIL [expected TIMEOUT] subtest: Navigating to a different document with link click
    • FAIL [expected NOTRUN] subtest: Navigating to a different document with form submission
  • OK /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling-dynamic.html (#28066)
    • FAIL [expected PASS] subtest: DE0D 0041 set in href="" targeting a frame and clicked
  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted
  • CRASH [expected OK] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • TIMEOUT [expected FAIL] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
    • FAIL [expected NOTRUN] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK /html/syntax/parsing/DOMContentLoaded-defer.html (#21550)
    • FAIL [expected PASS] subtest: The end: DOMContentLoaded and defer scripts
  • TIMEOUT /html/webappapis/scripting/events/compile-event-handler-settings-objects.html (#24246)
    • FAIL [expected PASS] subtest: The entry settings object while executing the compiled callback via Web IDL's invoke must be that of the node document
  • OK [expected TIMEOUT] /webaudio/the-audio-api/the-analysernode-interface/realtimeanalyser-fft-scaling.html (#21743)
  • TIMEOUT [expected OK] /webmessaging/without-ports/017.html (#24486)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank
  • OK [expected TIMEOUT] /webmessaging/without-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
  • ERROR [expected OK] /workers/constructors/Worker/Worker-constructor.html (#22991)

@bors-servo
Copy link
Contributor

☀️ Test successful - checks-github
Approved by: mukilan
Pushing 45000be to master...

@bors-servo bors-servo merged commit 45000be into master Apr 13, 2023
@bors-servo bors-servo deleted the fix-nightly-job-upload-error branch April 13, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrobinson mrobinson mrobinson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mukilan @bors-servo @mrobinson