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

Handle jlap response without request e.g. for file:/// #12994

Merged
merged 7 commits into from
Aug 22, 2023
Merged

Conversation

dholth
Copy link
Contributor

@dholth dholth commented Aug 10, 2023

Description

Fix #12963

Test needs to be updated to point to the same place the test http server is getting its files; do we need to test with and without .jlap on the filesystem; do we work correctly when there is a range request or do we detect that those are unsupported; or should we simply not do the jlap bits when we are not on http or s3.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@dholth dholth requested a review from a team as a code owner August 10, 2023 15:19
@dholth dholth changed the title handle response without request e.g. for file:/// handle jlap response without request e.g. for file:/// Aug 10, 2023
@dholth dholth marked this pull request as draft August 10, 2023 15:19
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Aug 10, 2023
@dholth dholth marked this pull request as ready for review August 16, 2023 15:14
@jezdez jezdez changed the title handle jlap response without request e.g. for file:/// Handle jlap response without request e.g. for file:/// Aug 22, 2023
@@ -278,6 +320,8 @@ def test_jlap_sought(
package_repository_base: Path,
):
"""Test that we try to fetch the .jlap file."""
(package_repository_base / "osx-64" / "repodata.jlap").unlink(missing_ok=True)
Copy link
Member

Choose a reason for hiding this comment

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

Is this just a precaution? Or does this point to a larger issue of test cleanup failing elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test web server thread points to a single directory. We could use other strategies like copying those files into a per-test directory or creating a per-test scoped fixture to cleanup.

@dholth dholth merged commit fa5c494 into main Aug 22, 2023
67 checks passed
@dholth dholth deleted the jlap-fetch-file branch August 22, 2023 11:50
@jezdez jezdez mentioned this pull request Sep 26, 2023
92 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

jlap: error with file:/// repositories
3 participants