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 test_http.request_callback #828

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Conversation

ddelange
Copy link
Contributor

@ddelange ddelange commented Jul 24, 2024

Title

Fix tests with urllib3 v2

Motivation

Fix #784, CI was running only 3.7 (hence urllib3 v1) due to a typo regarding python-version.

Tests

Fix sftp tests now that CI is running python 3.8--3.12

Work in progress

Checklist

Before you create the PR, please make sure you have:

  • Picked a concise, informative and complete title
  • Clearly explained the motivation behind the PR
  • Linked to any existing issues that your PR will be solving
  • Included tests for any new functionality
  • Checked that all unit tests pass

Workflow

Please avoid rebasing and force-pushing to the branch of the PR once a review is in progress.
Rebasing can make your commits look a bit cleaner, but it also makes life more difficult from the reviewer, because they are no longer able to distinguish between code that has already been reviewed, and unreviewed code.

@ddelange ddelange force-pushed the patch-5 branch 4 times, most recently from f4bac24 to a874df5 Compare July 24, 2024 10:32
@ddelange ddelange mentioned this pull request Jul 24, 2024
3 tasks
@ddelange ddelange force-pushed the patch-5 branch 12 times, most recently from 5671d0e to 9e6f31c Compare July 24, 2024 13:13
@ddelange
Copy link
Contributor Author

ddelange commented Jul 24, 2024

with the new python versions in the test matrix, ssl in integration test started complaining about ftps server's self-signed cert

ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate (_ssl.c:1131)

so i explicitly created a self-signed cert for localhost and passed it in the integration test. but now there's

ERROR    smart_open.ftp:ftp.py:96 Unable to connect to FTP server: try checking the host and port!

and i'm not sure how to proceed.

@ddelange
Copy link
Contributor Author

ddelange commented Jul 24, 2024

@RachitSharma2001 as author of these files, do you have any idea?

@ddelange
Copy link
Contributor Author

got it, we're green now:)

  rsa_cert_file=/etc/vsftpd.pem
+ rsa_private_key_file=/etc/vsftpd.key

@ddelange ddelange force-pushed the patch-5 branch 2 times, most recently from 8bdd9d3 to bfb9f93 Compare July 25, 2024 08:06
@ddelange
Copy link
Contributor Author

@mpenkov please review

from functools import partial

# localhost has self-signed cert, see ci_helpers/helpers.sh:create_ftp_ftps_servers
ssl.create_default_context = partial(ssl.create_default_context, cafile="/etc/vsftpd.pem")
Copy link
Contributor Author

@ddelange ddelange Aug 9, 2024

Choose a reason for hiding this comment

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

probably worth moving to transport_params in a separate PR https://docs.python.org/3/library/ssl.html#ssl.create_default_context

@ddelange ddelange mentioned this pull request Aug 15, 2024
5 tasks
@mpenkov
Copy link
Collaborator

mpenkov commented Aug 15, 2024

LGTM, thank you!

@mpenkov mpenkov merged commit 8a58abe into piskvorky:develop Aug 15, 2024
26 checks passed
ddelange added a commit to ddelange/smart_open that referenced this pull request Aug 15, 2024
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.

Test failures with urllib3 2.0.4
2 participants