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

Fix S3 build on Windows; enable S3 build w/out test #1474

Merged
merged 3 commits into from
Jan 14, 2020

Conversation

ihnorton
Copy link
Member

@ihnorton ihnorton commented Jan 14, 2020

This PR fixes the S3 build on Windows, which was broken due to header reordering in #1463, which created a conflict with a Windows.h definition of GetMessage -> GetMessageA (x-ref solution in aws/aws-sdk-cpp#402).

The breakage was not noticed on CI because we do not currently run a build with S3 enabled on Windows, due to the S3 tests timing out when running against minio on Windows. I've verified that minio is running on CI, with a python script that queries the minio live and ready pages (ensures status 200). CI logs also indicate that a number of tests requiring S3 do pass (tests which would fail without minio running).

Locally, the tests finish successfully in a few minutes on my Windows laptop. I'm still unclear on the reason for the slowdown on azure.

For now, to catch future S3-related build issues, this PR enables TILEDB_S3 for the windows builds, but disables the S3 compiler definition before building the tests, which causes the tests to run in the local configuration.

ihnorton and others added 3 commits January 13, 2020 15:01
Tests time out running against minio on azure, for unclear reasons.
Several of the dense tests run for 11+ minutes, and the compression
tests take 30-60s each.

Move tiledb-install to build step
@cngzhnp
Copy link
Contributor

cngzhnp commented Jan 14, 2020

Hello @ihnorton,
Sorry about inconsistency in order to fix header inclusion in that case. I did not realize that anything could be broken on S3 as a side effect.

@ihnorton ihnorton merged commit 598f379 into TileDB-Inc:dev Jan 14, 2020
@ihnorton
Copy link
Member Author

ihnorton commented Jan 14, 2020

@cngzhnp it was not caught by the tests/CI, so no worries. Thanks again for the contribution.

@ihnorton ihnorton deleted the ihn/fix_win_s3 branch January 14, 2020 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants