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

JonathanHenson · 2023-10-03T23:48:39Z

…hout failing if there is no compiler or to use the system loader.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…hout failing if there is no compiler or to use the system loader.

graebm · 2023-10-10T01:07:42Z

.github/workflows/ci.yml

+    - name: Non Strict/System Mode
+      run: |
+        ./codebuild/test-sys-install-mode.sh    


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 .

graebm · 2023-10-10T03:50:25Z

awscrt/__init__.py

@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: Apache-2.0.

 from weakref import WeakSet
+from .config import crt_wheel_installed


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

graebm · 2023-10-10T04:10:46Z

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.
+
+````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.


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() ?

graebm · 2023-10-10T04:21:22Z

README.md

@@ -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


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 ``

graebm · 2023-10-10T04:32:36Z

README.md

+
+````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.
+
+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.


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

graebm · 2023-10-10T04:40:10Z

setup.py

+    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',
+    )


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)

graebm · 2023-10-10T04:43:53Z

setup.py

+        test_suite='test',
+    )
+
+except (SystemExit, Exception) as e:


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

graebm · 2023-10-10T04:58:02Z

README.md

@@ -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
+
+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.


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

JonathanHenson added 15 commits October 3, 2023 16:47

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

b160841

…hout failing if there is no compiler or to use the system loader.

Fix malformed yaml.

e8f08b2

Lets see if we can get it right this time.

adfc3c2

try using venv

e25f26f

make sure all the python tools are installed.

d151f5a

Actually make my change this time.

ca34baf

Saved some unsaved files and added the config.py back to the git repo.

6156ead

temp commit to see what's going on.

592cc93

Ah.... systemexit

aa8c1b8

use pip install to hopefully get past new python linter failures.

69e198a

fix autopep8 linter.

bc6033e

white space removal.

1104cd6

lol, we actually have a script for this.

12bc7ba

Update some docs.

c625d3e

remove unused function.

19d272d

graebm reviewed Oct 10, 2023

View reviewed changes

jmklix closed this May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

JonathanHenson commented Oct 3, 2023

graebm Oct 10, 2023

graebm Oct 10, 2023

graebm Oct 10, 2023

graebm Oct 10, 2023

graebm Oct 10, 2023

graebm Oct 10, 2023

graebm Oct 10, 2023

graebm Oct 10, 2023


		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.

		````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.


		````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.

		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.

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

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

Conversation

JonathanHenson commented Oct 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment