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

Add a mode to allow crt to either be used as a default dependency wit… #505

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ env:
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
AWS_DEFAULT_REGION: ${{ secrets.AWS_DEFAULT_REGION }}
AWS_REGION: us-east-1
AWS_CRT_BUILD_STRICT_MODE: ON

jobs:
manylinux1:
Expand Down Expand Up @@ -265,6 +266,14 @@ jobs:
python3 -m pip install -v awscrt-1.0.0.dev0.tar.gz
python3 -c "import awscrt.io"

non-strict-system-mode:
runs-on: ubuntu-22.04 #latest
steps:
- uses: actions/checkout@v3
- name: Non Strict/System Mode
run: |
./codebuild/test-sys-install-mode.sh
Comment on lines +273 to +275
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: Just enter the shell commands here. It's simpler

Suggested change
- name: Non Strict/System Mode
run: |
./codebuild/test-sys-install-mode.sh
- name: Non Strict/System Mode
env:
AWS_CRT_BUILD_STRICT_MODE: OFF
run: |
python3 -m pip install setuptools wheel --upgrade
python3 -m pip install -v .


# check that docs can still build
check-docs:
runs-on: ubuntu-22.04 # latest
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -531,3 +531,4 @@ deps/

# API docs are updated automatically by .github/workflows/docs.yml
docs/

8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,13 @@ Please note that on Mac, once a private key is used with a certificate, that cer
static: certificate has an existing certificate-key pair that was previously imported into the Keychain. Using key from Keychain instead of the one provided.
```

## Using With a System-Level Installation of the _awscrt Module or as an Optional Dependency
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know much about package managers, but I can't imagine one vending _awscrt.so without the rest of the awscrt python package. Maybe don't bother mentioning use cases beyond: to support awscrt as an optional-but-suggested dependency.

Just FYI, here's what's in the Amazon Linux 2023 package:

$ dnf repoquery -l python3-awscrt
/usr/lib64/python3.9/site-packages/_awscrt.cpython-39-x86_64-linux-gnu.so
/usr/lib64/python3.9/site-packages/awscrt
/usr/lib64/python3.9/site-packages/awscrt-0.16.7.dist-info
/usr/lib64/python3.9/site-packages/awscrt-0.16.7.dist-info/INSTALLER
/usr/lib64/python3.9/site-packages/awscrt-0.16.7.dist-info/LICENSE
/usr/lib64/python3.9/site-packages/awscrt-0.16.7.dist-info/METADATA
/usr/lib64/python3.9/site-packages/awscrt-0.16.7.dist-info/NOTICE
/usr/lib64/python3.9/site-packages/awscrt-0.16.7.dist-info/WHEEL
/usr/lib64/python3.9/site-packages/awscrt-0.16.7.dist-info/top_level.txt
/usr/lib64/python3.9/site-packages/awscrt/__init__.py
/usr/lib64/python3.9/site-packages/awscrt/__pycache__
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/__init__.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/__init__.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/_test.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/_test.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/auth.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/auth.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/checksums.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/checksums.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/common.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/common.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/crypto.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/crypto.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/exceptions.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/exceptions.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/http.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/http.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/io.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/io.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/mqtt.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/mqtt.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/mqtt5.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/mqtt5.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/s3.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/s3.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/websocket.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/__pycache__/websocket.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/_test.py
/usr/lib64/python3.9/site-packages/awscrt/auth.py
/usr/lib64/python3.9/site-packages/awscrt/checksums.py
/usr/lib64/python3.9/site-packages/awscrt/common.py
/usr/lib64/python3.9/site-packages/awscrt/crypto.py
/usr/lib64/python3.9/site-packages/awscrt/eventstream
/usr/lib64/python3.9/site-packages/awscrt/eventstream/__init__.py
/usr/lib64/python3.9/site-packages/awscrt/eventstream/__pycache__
/usr/lib64/python3.9/site-packages/awscrt/eventstream/__pycache__/__init__.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/eventstream/__pycache__/__init__.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/eventstream/__pycache__/rpc.cpython-39.opt-1.pyc
/usr/lib64/python3.9/site-packages/awscrt/eventstream/__pycache__/rpc.cpython-39.pyc
/usr/lib64/python3.9/site-packages/awscrt/eventstream/rpc.py
/usr/lib64/python3.9/site-packages/awscrt/exceptions.py
/usr/lib64/python3.9/site-packages/awscrt/http.py
/usr/lib64/python3.9/site-packages/awscrt/io.py
/usr/lib64/python3.9/site-packages/awscrt/mqtt.py
/usr/lib64/python3.9/site-packages/awscrt/mqtt5.py
/usr/lib64/python3.9/site-packages/awscrt/s3.py
/usr/lib64/python3.9/site-packages/awscrt/websocket.py
/usr/share/doc/python3-awscrt
/usr/share/doc/python3-awscrt/README.md
``


By default, upon installation if both the wheel is unavailable from PyPi and setup.py cannot build the _awscrt module, installation will still succeed. This is intended to allow the crt to be installed into other libraries or applications as a library without failing its install step if a compiler or any other required build tool is not available. This also allows _awscrt to be loaded at the system level via. operating system package managers as this will cause the loader to look at the system path when it can't find the locally installed module.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ask our Python folks if this is a good approach. Failing at run time instead of install time is a pretty radical change. I want to be sure we have buy-in from people who know the community best


````awscrt.crt_wheel_installed()```` will always return without throwing an Exception even if the build itself failed. If this function returns true, the rest of the modules in this package may be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

pedantic: A "wheel" is whole installable package, not just the C-extension. Even packages without C-extensions, or anything platform specific, still have wheels in PyPI. For example:
botocore-1.31.62-py3-none-any.whl

Maybe name it: c_extension_installed() ? is_fully_installed() ?


To disable this behavior at build time, set ```AWS_CRT_BUILD_STRICT_MODE=ON``` which will cause the install to abort if an error occurs building the wheel.
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: we have a few on/off env variables already, and they all use 1 instead of ON


## Crash Handler
You can enable the crash handler by setting the environment variable `AWS_CRT_CRASH_HANDLER=1`. This will print the callstack to `stderr` in the event of a fatal error.
1 change: 1 addition & 0 deletions awscrt/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# SPDX-License-Identifier: Apache-2.0.

from weakref import WeakSet
from .config import crt_wheel_installed
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: I would just put the function here in __init__.py

or name the other file _config.py so it's not a new official public namespace


__all__ = [
'auth',
Expand Down
16 changes: 16 additions & 0 deletions awscrt/config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
"""
Config module which can always be checked regardless of if the CRT was successfully built, installed on system, or not.
"""

"""
Returns True if either _awscrt was installed in the local environment or loaded from the system.
Otherwise returns False indicating that any other functions or modules in this library will also fail.
"""


def crt_wheel_installed():
try:
import _awscrt
return True
except Exception:
return False
1 change: 1 addition & 0 deletions codebuild/cd/manylinux-x64-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ phases:
pre_build:
commands:
- export CC=gcc
- export AWS_CRT_BUILD_STRICT_MODE=ON
- cd aws-crt-python
- /opt/python/cp37-cp37m/bin/python ./continuous-delivery/update-version.py
build:
Expand Down
1 change: 1 addition & 0 deletions codebuild/cd/manylinux-x86-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ phases:
pre_build:
commands:
- export CC=gcc
- export AWS_CRT_BUILD_STRICT_MODE=ON
- cd aws-crt-python
- /opt/python/cp37-cp37m/bin/python ./continuous-delivery/update-version.py
build:
Expand Down
2 changes: 2 additions & 0 deletions codebuild/cd/manylinux1-tee.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ phases:
pre_build:
commands:
- export CC=gcc
- export AWS_CRT_BUILD_STRICT_MODE=ON

build:
commands:
- echo Build started on `date`
Expand Down
8 changes: 8 additions & 0 deletions codebuild/test-config-on-failure.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0.

import awscrt

print('Is CRT wheel installed? {}', awscrt.crt_wheel_installed())
## this had better be False since the caller sabotaged the build in some way.
assert(awscrt.crt_wheel_installed() == False)
13 changes: 13 additions & 0 deletions codebuild/test-sys-install-mode.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/bash

set -euxo pipefail

# intentionally not pulling the submodules to make the build fail cleanly.
# git submodule update --init

export AWS_CRT_BUILD_STRICT_MODE=OFF
python -m venv ./crt-build
source ./crt-build/bin/activate
pip install setuptools wheel --upgrade
pip install -v ./
python codebuild/test-config-on-failure.py
2 changes: 2 additions & 0 deletions continuous-delivery/build-wheels-manylinux2014-aarch64.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#assumes image based on manylinux2014 + extras (cmake3, libcrypto, etc)
set -ex

export AWS_CRT_BUILD_STRICT_MODE=ON

/opt/python/cp39-cp39/bin/python ./continuous-delivery/update-version.py

/opt/python/cp37-cp37m/bin/python setup.py sdist bdist_wheel
Expand Down
1 change: 1 addition & 0 deletions continuous-delivery/build-wheels-manylinux2014-x86_64.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/bin/bash
#assumes image based on manylinux2014 + extras (cmake3, libcrypto, etc)
set -ex
export AWS_CRT_BUILD_STRICT_MODE=ON

/opt/python/cp39-cp39/bin/python ./continuous-delivery/update-version.py

Expand Down
2 changes: 2 additions & 0 deletions continuous-delivery/build-wheels-win64.bat
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
set AWS_CRT_BUILD_STRICT_MODE=ON

"C:\Program Files\Python39\python.exe" continuous-delivery\update-version.py || goto error

"C:\Program Files\Python37\python.exe" setup.py sdist bdist_wheel || goto error
Expand Down
77 changes: 55 additions & 22 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ def using_system_libcrypto():
return os.getenv('AWS_CRT_BUILD_USE_SYSTEM_LIBCRYPTO') == '1'


def strict_build_success_mode():
strict_mode = os.getenv('AWS_CRT_BUILD_STRICT_MODE')
print("Strict-Mode is set to: {}".format(strict_mode))
return strict_mode is not None and strict_mode.strip().lower() != 'off'


class AwsLib:
def __init__(self, name, extra_cmake_args=[], libname=None):
self.name = name
Expand Down Expand Up @@ -391,25 +397,52 @@ def _load_version():
return VERSION_RE.match(fp.read()).group(1)


setuptools.setup(
name="awscrt",
version=_load_version(),
license="Apache 2.0",
author="Amazon Web Services, Inc",
author_email="[email protected]",
description="A common runtime for AWS Python projects",
long_description=_load_readme(),
long_description_content_type='text/markdown',
url="https://github.com/awslabs/aws-crt-python",
# Note: find_packages() without extra args will end up installing test/
packages=setuptools.find_packages(include=['awscrt*']),
classifiers=[
"Programming Language :: Python :: 3",
"License :: OSI Approved :: Apache Software License",
"Operating System :: OS Independent",
],
python_requires='>=3.7',
ext_modules=[awscrt_ext()],
cmdclass={'build_ext': awscrt_build_ext, "bdist_wheel": bdist_wheel_abi3},
test_suite='test',
)
try:
setuptools.setup(
name="awscrt",
version=_load_version(),
license="Apache 2.0",
author="Amazon Web Services, Inc",
author_email="[email protected]",
description="A common runtime for AWS Python projects",
long_description=_load_readme(),
long_description_content_type='text/markdown',
url="https://github.com/awslabs/aws-crt-python",
# Note: find_packages() without extra args will end up installing test/
packages=setuptools.find_packages(include=['awscrt*']),
classifiers=[
"Programming Language :: Python :: 3",
"License :: OSI Approved :: Apache Software License",
"Operating System :: OS Independent",
],
python_requires='>=3.7',
ext_modules=[awscrt_ext()],
cmdclass={'build_ext': awscrt_build_ext, "bdist_wheel": bdist_wheel_abi3},
test_suite='test',
)

except (SystemExit, Exception) as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

did you really encounter SystemExit? If so, please document what causes that. Usually, just catching Exception is good because it won't catch stuff you're not supposed to, like ctrl-C.

I asked BingGPT if setuptools.setup() could return SystemExit and it said that's what happens if someone calls your setup.py directly and doesn't pass the correct args, like: python3 setup.py asdf

if (strict_build_success_mode()):
raise e

# build without the c extension, and allow the system loader to pick it up if/when it's available.
# The config module will provide users with a way to check.
setuptools.setup(
name="awscrt",
version=_load_version(),
license="Apache 2.0",
author="Amazon Web Services, Inc",
author_email="[email protected]",
description="A common runtime for AWS Python projects",
long_description=_load_readme(),
long_description_content_type='text/markdown',
url="https://github.com/awslabs/aws-crt-python",
# Note: find_packages() without extra args will end up installing test/
packages=setuptools.find_packages(include=['awscrt*']),
classifiers=[
"Programming Language :: Python :: 3",
"License :: OSI Approved :: Apache Software License",
"Operating System :: OS Independent",
],
python_requires='>=3.7',
)
Comment on lines +430 to +448
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very copy/pasty. It's very hard to tell what's different, and it's likely someone makes a mistake one day and updates 1 block and not the other. Maybe break this into a function:

def setup(build_ext: bool):
    ...
    setuptools.setup(
        ...
    )


try:
    setup(build_ext=True)

except Exception as e:
    if (strict_build_success_mode()):
        raise e
    
    setup(build_ext=False)

Loading