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

Fix test_http.request_callback #828

Merged
merged 1 commit into from
Aug 15, 2024
+75 −78
Merged

Fix test_http.request_callback #828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
114 changes: 50 additions & 64 deletions .github/workflows/python-package.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
name: Test
on: [push, pull_request]
concurrency: # https://stackoverflow.com/questions/66335225#comment133398800_72408109
group: ${{ github.workflow }}-${{ github.ref || github.run_id }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
jobs:
linters:
runs-on: ubuntu-latest
Expand All @@ -12,10 +15,10 @@ jobs:
python-version: "3.11"

- name: Update pip
run: python -m pip install -U pip
run: pip install -U pip

- name: Install dependencies
run: python -m pip install flake8
run: pip install flake8

- name: Run flake8 linter (source)
run: flake8 --show-source smart_open
Expand All @@ -26,17 +29,17 @@ jobs:
strategy:
matrix:
include:
- {python: '3.8', os: ubuntu-20.04}
- {python: '3.9', os: ubuntu-20.04}
- {python: '3.10', os: ubuntu-20.04}
- {python: '3.11', os: ubuntu-20.04}
- {python: '3.12', os: ubuntu-20.04}

- {python: '3.8', os: windows-2019}
- {python: '3.9', os: windows-2019}
- {python: '3.10', os: windows-2019}
- {python: '3.11', os: windows-2019}
- {python: '3.12', os: windows-2019}
- {python-version: '3.8', os: ubuntu-20.04}
- {python-version: '3.9', os: ubuntu-20.04}
- {python-version: '3.10', os: ubuntu-20.04}
- {python-version: '3.11', os: ubuntu-20.04}
- {python-version: '3.12', os: ubuntu-20.04}

- {python-version: '3.8', os: windows-2019}
- {python-version: '3.9', os: windows-2019}
- {python-version: '3.10', os: windows-2019}
- {python-version: '3.11', os: windows-2019}
- {python-version: '3.12', os: windows-2019}
steps:
- uses: actions/checkout@v2

Expand All @@ -45,13 +48,7 @@ jobs:
python-version: ${{ matrix.python-version }}

- name: Update pip
run: python -m pip install -U pip

#
# https://askubuntu.com/questions/1428181/module-lib-has-no-attribute-x509-v-flag-cb-issuer-check
#
- name: Upgrade PyOpenSSL
run: python -m pip install pyOpenSSL --upgrade
run: pip install -U pip

- name: Install smart_open without dependencies
run: pip install -e .
Expand All @@ -71,20 +68,20 @@ jobs:
strategy:
matrix:
include:
- {python: '3.8', os: ubuntu-20.04}
- {python: '3.9', os: ubuntu-20.04}
- {python: '3.10', os: ubuntu-20.04}
- {python: '3.11', os: ubuntu-20.04}
- {python: '3.12', os: ubuntu-20.04}
- {python-version: '3.8', os: ubuntu-20.04}
- {python-version: '3.9', os: ubuntu-20.04}
- {python-version: '3.10', os: ubuntu-20.04}
- {python-version: '3.11', os: ubuntu-20.04}
- {python-version: '3.12', os: ubuntu-20.04}

#
# Some of the doctests don't pass on Windows because of Windows-specific
# character encoding issues.
#
# - {python: '3.7', os: windows-2019}
# - {python: '3.8', os: windows-2019}
# - {python: '3.9', os: windows-2019}
# - {python: '3.10', os: windows-2019}
# - {python-version: '3.7', os: windows-2019}
# - {python-version: '3.8', os: windows-2019}
# - {python-version: '3.9', os: windows-2019}
# - {python-version: '3.10', os: windows-2019}

steps:
- uses: actions/checkout@v2
Expand All @@ -94,10 +91,7 @@ jobs:
python-version: ${{ matrix.python-version }}

- name: Update pip
run: python -m pip install -U pip

- name: Upgrade PyOpenSSL
run: python -m pip install pyOpenSSL --upgrade
run: pip install -U pip

- name: Install smart_open and its dependencies
run: pip install -e .[test]
Expand All @@ -114,18 +108,18 @@ jobs:
strategy:
matrix:
include:
- {python: '3.8', os: ubuntu-20.04}
- {python: '3.9', os: ubuntu-20.04}
- {python: '3.10', os: ubuntu-20.04}
- {python: '3.11', os: ubuntu-20.04}
- {python: '3.12', os: ubuntu-20.04}
- {python-version: '3.8', os: ubuntu-20.04}
- {python-version: '3.9', os: ubuntu-20.04}
- {python-version: '3.10', os: ubuntu-20.04}
- {python-version: '3.11', os: ubuntu-20.04}
- {python-version: '3.12', os: ubuntu-20.04}

# Not sure why we exclude these, perhaps for historical reasons?
#
# - {python: '3.7', os: windows-2019}
# - {python: '3.8', os: windows-2019}
# - {python: '3.9', os: windows-2019}
# - {python: '3.10', os: windows-2019}
# - {python-version: '3.7', os: windows-2019}
# - {python-version: '3.8', os: windows-2019}
# - {python-version: '3.9', os: windows-2019}
# - {python-version: '3.10', os: windows-2019}

steps:
- uses: actions/checkout@v2
Expand All @@ -135,20 +129,17 @@ jobs:
python-version: ${{ matrix.python-version }}

- name: Update pip
run: python -m pip install -U pip

- name: Upgrade PyOpenSSL
run: python -m pip install pyOpenSSL --upgrade

- run: python -m pip install numpy
run: pip install -U pip

- name: Install smart_open and its dependencies
run: pip install -e .[test]

- run: bash ci_helpers/helpers.sh enable_moto_server
if: ${{ matrix.moto_server }}

- run: |
- name: Start vsftpd
timeout-minutes: 2
run: |
sudo apt-get install vsftpd
sudo bash ci_helpers/helpers.sh create_ftp_ftps_servers
Expand All @@ -169,16 +160,16 @@ jobs:
strategy:
matrix:
include:
- {python: '3.8', os: ubuntu-20.04}
- {python: '3.9', os: ubuntu-20.04}
- {python: '3.10', os: ubuntu-20.04}
- {python: '3.11', os: ubuntu-20.04}
- {python: '3.12', os: ubuntu-20.04}
- {python-version: '3.8', os: ubuntu-20.04}
- {python-version: '3.9', os: ubuntu-20.04}
- {python-version: '3.10', os: ubuntu-20.04}
- {python-version: '3.11', os: ubuntu-20.04}
- {python-version: '3.12', os: ubuntu-20.04}

# - {python: '3.7', os: windows-2019}
# - {python: '3.8', os: windows-2019}
# - {python: '3.9', os: windows-2019}
# - {python: '3.10', os: windows-2019}
# - {python-version: '3.7', os: windows-2019}
# - {python-version: '3.8', os: windows-2019}
# - {python-version: '3.9', os: windows-2019}
# - {python-version: '3.10', os: windows-2019}

steps:
- uses: actions/checkout@v2
Expand All @@ -188,16 +179,11 @@ jobs:
python-version: ${{ matrix.python-version }}

- name: Update pip
run: python -m pip install -U pip

- name: Upgrade PyOpenSSL
run: python -m pip install pyOpenSSL --upgrade
run: pip install -U pip

- name: Install smart_open and its dependencies
run: pip install -e .[test]

- run: pip install awscli pytest_benchmark

- name: Run benchmarks
run: python ci_helpers/run_benchmarks.py
env:
Expand Down
5 changes: 5 additions & 0 deletions ci_helpers/helpers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ create_ftp_ftps_servers(){
mkdir $home_dir
useradd -p $(echo $pass | openssl passwd -1 -stdin) -d $home_dir $user
chown $user:$user $home_dir
openssl req -x509 -nodes -new -sha256 -days 10240 -newkey rsa:2048 -keyout /etc/vsftpd.key -out /etc/vsftpd.pem -subj "/C=ZA/CN=localhost"
chmod 755 /etc/vsftpd.key
chmod 755 /etc/vsftpd.pem

server_setup='''
listen=YES
Expand All @@ -32,6 +35,8 @@ chroot_local_user=YES
allow_writeable_chroot=YES'''

additional_ssl_setup='''
rsa_cert_file=/etc/vsftpd.pem
rsa_private_key_file=/etc/vsftpd.key
ssl_enable=YES
allow_anon_ssl=NO
force_local_data_ssl=NO
Expand Down
7 changes: 6 additions & 1 deletion integration-tests/test_ftp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from __future__ import unicode_literals
import pytest
from smart_open import open
import ssl
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 •
edited
Loading

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



@pytest.fixture(params=[("ftp", 21), ("ftps", 90)])
Expand Down Expand Up @@ -81,4 +86,4 @@ def test_line_endings_binary(server_info):

with open(f"{server_type}://user:123@localhost:{port_num}/file4", "rb") as f:
for line in f:
assert B_CLRF in line
assert B_CLRF in line
5 changes: 4 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,12 @@ def read(fname):
tests_require = all_deps + [
'moto[server]',
'responses',
'boto3',
'pytest',
'pytest-rerunfailures',
'pytest_benchmark',
'awscli',
'pyopenssl',
'numpy',
]

setup(
Expand Down
2 changes: 1 addition & 1 deletion smart_open/ftp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def convert_transport_params_to_args(transport_params):
def _connect(hostname, username, port, password, secure_connection, transport_params):
kwargs = convert_transport_params_to_args(transport_params)
if secure_connection:
ssl_context = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH)
ssl_context = ssl.create_default_context(purpose=ssl.Purpose.SERVER_AUTH)
ftp = FTP_TLS(context=ssl_context, **kwargs)
else:
ftp = FTP(**kwargs)
Expand Down
20 changes: 9 additions & 11 deletions smart_open/tests/test_http.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,22 @@
URL = 'http://localhost'
HTTPS_URL = 'https://localhost'
HEADERS = {
'Content-Length': str(len(BYTES)),
'Accept-Ranges': 'bytes',
}


def request_callback(request, headers=HEADERS, data=BYTES):
try:
range_string = request.headers['range']
except KeyError:
return (200, headers, data)
headers = headers.copy()
range_string = request.headers.get('range', 'bytes=0-')

start, end = range_string.replace('bytes=', '').split('-', 1)
start, end = range_string.replace('bytes=', '', 1).split('-', 1)
start = int(start)
if end:
end = int(end)
else:
end = len(data)
return (200, headers, data[start:end])
end = int(end) if end else len(data)

data = data[start:end]
headers['Content-Length'] = str(len(data))

return (200, headers, data)


@unittest.skipIf(os.environ.get('TRAVIS'), 'This test does not work on TravisCI for some reason')
Expand Down
Loading