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

Add no_sign_request to s3 read test #778

Closed
wants to merge 3 commits into from
Closed

Add no_sign_request to s3 read test #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26d60cb
Add no_sign_request to s3 read test
gspowley Sep 27, 2024
46e8db6
fix windows build
gspowley Sep 27, 2024
ce4f468
pin m2w64-htslib=1.19
gspowley Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apis/python/tests/test_tiledbvcf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,12 @@ def test_sample_and_region_partitioned_read():
def test_large_export_correctness():
uri = "s3://tiledb-inc-demo-data/tiledbvcf-arrays/v4/vcf-samples-20"

ds = tiledbvcf.Dataset(uri)
tiledb_config = {
"vfs.s3.no_sign_request": True,
"vfs.s3.region": "us-east-1",
}

ds = tiledbvcf.Dataset(uri, tiledb_config=tiledb_config)
df = ds.read(
attrs=[
"sample_name",
Expand Down
2 changes: 1 addition & 1 deletion ci/gha-win-env.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ dependencies:
# build libtiledbvcf
- cmake
- git
- m2w64-htslib
- m2w64-htslib=1.19
- tiledb=2.15
- vs2019_win-64
# build tiledbvcf-py
Expand Down
2 changes: 1 addition & 1 deletion libtiledbvcf/cmake/Modules/FindHTSlib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ if (NOT HTSLIB_FOUND)
ExternalProject_Add(ep_htslib
PREFIX "externals"
URL https://github.com/TileDB-Inc/m2w64-htslib-build/releases/download/1.20-0/m2w64-htslib-1.20-0.tar.gz
URL_HASH SHA1=da39a3ee5e6b4b0d3255bfef95601890afd80709
Copy link
Member

Choose a reason for hiding this comment

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

Why did the mismatch happen? We should investigate it before changing the hash.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unclear exactly how the mismatch was introduced. It was added in #746

But it's been known to be a problem for about a month now. I first saw it in #763, which I then also used in #766

Copy link
Collaborator

Choose a reason for hiding this comment

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

This change shouldn't be needed for this PR though. That Windows CI has m2w64-htslib installed from conda. For some reason, in this PR, conda installs m2w64-htslib 1.20, which CMake can't find -- Could NOT find HTSlib (missing: HTSLIB_LIBRARIES HTSLIB_INCLUDE_DIR)

Whereas the last successful Windows CI build from last week installed m2w64-htslib 1.19 with conda, which CMake was able to find -- Found HTSlib: C:/Users/runneradmin/micromamba/envs/win-env/Library/lib/hts-3.lib

So 2 puzzles:

  1. Why is conda installing m2w64-1.20 this week but 1.19 just last week?
  2. Why can't CMake find htslib 1.20?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As a workaround if we can't quickly figure this out, we could pin m2w64-htslib to 1.19

TileDB-VCF/ci/gha-win-env.yml

Line 10 in 2271db0

- m2w64-htslib

Copy link
Collaborator

Choose a reason for hiding this comment

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

The conda binary for 1.20 is smaller compared to 1.19:

image

However, I downloaded the conda binary, and it still has Library/lib/hts-3.lib, so I would think that CMake should be able to find it.

Copy link
Member Author

@gspowley gspowley Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The existing hash for m2w64-htslib-1.20-0.tar.gz was wrong.

da39a3ee5e6b4b0d3255bfef95601890afd80709 is the hash from downloading the wrong URL.

$ wget -O- https://github.com/TileDB-Inc/m2w64-htslib-build/releases/download/foobar | sha1sum 
--2024-09-27 15:51:28--  https://github.com/TileDB-Inc/m2w64-htslib-build/releases/download/foobar
Resolving github.com (github.com)... 140.82.114.4
Connecting to github.com (github.com)|140.82.114.4|:443... connected.
HTTP request sent, awaiting response... 404 Not Found
2024-09-27 15:51:29 ERROR 404: Not Found.

da39a3ee5e6b4b0d3255bfef95601890afd80709  -

URL_HASH SHA1=6f3e208ccc0262f89dcdf344d96e40696a5db133
UPDATE_COMMAND ""
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
Expand Down
Loading