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 https connection on windows #2182

Merged
merged 2 commits into from
Feb 18, 2025
Merged

Conversation

phoebusm
Copy link
Collaborator

@phoebusm phoebusm commented Feb 14, 2025

Reference Issues/PRs

https://man312219.monday.com/boards/7852509418/pulses/8485223765

What does this implement or fix?

  1. Change s3 persistence test endpoint to https
  2. Patch aws sdk on pypi build as temporary fix for WinHttp TLS is completely disabled if m_verifySSL is false aws/aws-sdk-cpp#3008
  3. Upgrade aws sdk to the version specified in vcpkg.json

For conda, patch will be submitted upstream

Change Type (Required)

  • Patch (Bug fix or non-breaking improvement)
  • Minor (New feature, but backward compatible)
  • Major (Breaking changes)
  • Cherry pick

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

@phoebusm phoebusm force-pushed the fix/s3_windows_https_conn branch from 3cfda12 to bad019d Compare February 17, 2025 12:03
@phoebusm phoebusm marked this pull request as ready for review February 17, 2025 12:04
cpp/vcpkg.json Outdated
@@ -109,7 +109,7 @@
"overrides": [
{ "name": "openssl", "version-string": "3.3.0" },
{ "name": "arrow", "version": "18.0.0" },
{ "name": "aws-sdk-cpp", "version": "1.11.474" },
{ "name": "aws-sdk-cpp", "version": "1.11.474", "$note": "Update overlay json to upgrade" },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you put a note here warning people not to upgrade to 1.11.486+ yet?

aws/aws-sdk-cpp#3253

@phoebusm
Copy link
Collaborator Author

@phoebusm
Copy link
Collaborator Author

Upstream change for conda: aws/aws-sdk-cpp#3304

@github-actions github-actions bot added the patch Small change, should increase patch version label Feb 18, 2025
@phoebusm
Copy link
Collaborator Author

@phoebusm phoebusm merged commit dc0d813 into master Feb 18, 2025
153 of 154 checks passed
@phoebusm phoebusm deleted the fix/s3_windows_https_conn branch February 18, 2025 14:23
phoebusm added a commit that referenced this pull request Feb 18, 2025
#### Reference Issues/PRs
<!--Example: Fixes #1234. See also #3456.-->
https://man312219.monday.com/boards/7852509418/pulses/8485223765

#### What does this implement or fix?
1. Change s3 persistence test endpoint to `https`
2. Patch aws sdk on pypi build as temporary fix for
aws/aws-sdk-cpp#3008
3. Upgrade aws sdk to the version specified in `vcpkg.json`

For conda, patch will be submitted upstream

## Change Type (Required)
- [x] **Patch** (Bug fix or non-breaking improvement)
- [ ] **Minor** (New feature, but backward compatible)
- [ ] **Major** (Breaking changes)
- [ ] **Cherry pick**

#### Any other comments?

#### Checklist

<details>
  <summary>
   Checklist for code changes...
  </summary>
 
- [ ] Have you updated the relevant docstrings, documentation and
copyright notice?
- [ ] Is this contribution tested against [all ArcticDB's
features](../docs/mkdocs/docs/technical/contributing.md)?
- [ ] Do all exceptions introduced raise appropriate [error
messages](https://docs.arcticdb.io/error_messages/)?
 - [ ] Are API changes highlighted in the PR description?
- [ ] Is the PR labelled as enhancement or bug so it appears in
autogenerated release notes?
</details>

<!--
Thanks for contributing a Pull Request to ArcticDB! Please ensure you
have taken a look at:
- ArcticDB's Code of Conduct:
https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md
- ArcticDB's Contribution Licensing:
https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing
-->
phoebusm added a commit that referenced this pull request Feb 18, 2025
…elease branch (#2187)

#### Reference Issues/PRs
<!--Example: Fixes #1234. See also #3456.-->
Cherrypicking #2182 to v5.2.4 release branch

#### What does this implement or fix?
1. Change s3 persistence test endpoint to `https`
2. Patch aws sdk on pypi build as temporary fix for
aws/aws-sdk-cpp#3008
3. Upgrade aws sdk to the version specified in `vcpkg.json`

For conda, patch will be submitted upstream

## Change Type (Required)
- [x] **Patch** (Bug fix or non-breaking improvement)
- [ ] **Minor** (New feature, but backward compatible)
- [ ] **Major** (Breaking changes)
- [ ] **Cherry pick**

#### Any other comments?

#### Checklist

<details>
  <summary> Checklist for code changes... </summary>
 
- [ ] Have you updated the relevant docstrings, documentation and
copyright notice?
- [ ] Is this contribution tested against [all ArcticDB's
features](../docs/mkdocs/docs/technical/contributing.md)?
- [ ] Do all exceptions introduced raise appropriate [error
messages](https://docs.arcticdb.io/error_messages/)?
 - [ ] Are API changes highlighted in the PR description?
- [ ] Is the PR labelled as enhancement or bug so it appears in
autogenerated release notes?
</details>

<!--
Thanks for contributing a Pull Request to ArcticDB! Please ensure you
have taken a look at:
- ArcticDB's Code of Conduct:
https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md
- ArcticDB's Contribution Licensing:
https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing
-->
jamesmunro pushed a commit that referenced this pull request Feb 25, 2025
#### Reference Issues/PRs
<!--Example: Fixes #1234. See also #3456.-->
https://man312219.monday.com/boards/7852509418/pulses/8485223765

#### What does this implement or fix?
1. Change s3 persistence test endpoint to `https`
2. Patch aws sdk on pypi build as temporary fix for
aws/aws-sdk-cpp#3008
3. Upgrade aws sdk to the version specified in `vcpkg.json`

For conda, patch will be submitted upstream

## Change Type (Required)
- [x] **Patch** (Bug fix or non-breaking improvement)
- [ ] **Minor** (New feature, but backward compatible)
- [ ] **Major** (Breaking changes)
- [ ] **Cherry pick**

#### Any other comments?

#### Checklist

<details>
  <summary>
   Checklist for code changes...
  </summary>
 
- [ ] Have you updated the relevant docstrings, documentation and
copyright notice?
- [ ] Is this contribution tested against [all ArcticDB's
features](../docs/mkdocs/docs/technical/contributing.md)?
- [ ] Do all exceptions introduced raise appropriate [error
messages](https://docs.arcticdb.io/error_messages/)?
 - [ ] Are API changes highlighted in the PR description?
- [ ] Is the PR labelled as enhancement or bug so it appears in
autogenerated release notes?
</details>

<!--
Thanks for contributing a Pull Request to ArcticDB! Please ensure you
have taken a look at:
- ArcticDB's Code of Conduct:
https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md
- ArcticDB's Contribution Licensing:
https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Small change, should increase patch version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants