From 14efed919af2499c7dedb1120298b5361e96702b Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 11:51:17 -0700 Subject: [PATCH 01/25] initial wip --- awscrt/s3.py | 15 +++++++++++---- source/s3_meta_request.c | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index 816a718ae..6676fcb3d 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -302,7 +302,8 @@ def make_request( on_headers=None, on_body=None, on_done=None, - on_progress=None): + on_progress=None, + network_interface_names=None): """Create the Request to the the S3 server, :attr:`~S3RequestType.GET_OBJECT`/:attr:`~S3RequestType.PUT_OBJECT` requests are split it into multi-part requests under the hood for acceleration. @@ -449,6 +450,8 @@ def make_request( * `**kwargs` (dict): Forward-compatibility kwargs. + network_interface_names: (Optional[list(str)]) + Returns: S3Request """ @@ -468,7 +471,8 @@ def make_request( on_body=on_body, on_done=on_done, on_progress=on_progress, - region=self._region) + region=self._region, + network_interface_names=network_interface_names) class S3Request(NativeResource): @@ -505,7 +509,8 @@ def __init__( on_body=None, on_done=None, on_progress=None, - region=None): + region=None, + network_interface_names=None): assert isinstance(client, S3Client) assert isinstance(request, HttpRequest) assert callable(on_headers) or on_headers is None @@ -513,6 +518,7 @@ def __init__( assert callable(on_done) or on_done is None assert isinstance(part_size, int) or part_size is None assert isinstance(multipart_upload_threshold, int) or multipart_upload_threshold is None + assert isinstance(network_interface_names, list) and all(isinstance(name, str) for name in network_interface_names) or network_interface_names is None if type == S3RequestType.DEFAULT and not operation_name: raise ValueError("'operation_name' must be set when using S3RequestType.DEFAULT") @@ -564,7 +570,8 @@ def __init__( validate_response_checksum, part_size, multipart_upload_threshold, - s3_request_core) + s3_request_core, + network_interface_names) @property def finished_future(self): diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 7586537e5..c4c68b542 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -377,9 +377,11 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { uint64_t part_size; /* K */ uint64_t multipart_upload_threshold; /* K */ PyObject *py_core; /* O */ + PyObject *network_interface_names_py; /* O */ + if (!PyArg_ParseTuple( args, - "OOOizOOzzs#iipKKO", + "OOOizOOzzs#iipKKOO", &py_s3_request, &s3_client_py, &http_request_py, @@ -396,7 +398,8 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { &validate_response_checksum, &part_size, &multipart_upload_threshold, - &py_core)) { + &py_core, + &network_interface_names_py)) { return NULL; } struct aws_s3_client *s3_client = aws_py_get_s3_client(s3_client_py); @@ -438,6 +441,19 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { .validate_response_checksum = validate_response_checksum != 0, }; + struct aws_byte_cursor *network_interface_names = NULL; + int num_network_interface_names = 0; + + if (network_interface_names_py != Py_None) { + if (!PyList_Check(network_interface_names_py)) { + // waahm7: todo, correct way to raise errors? + PyErr_SetString(PyExc_TypeError, "Expected a list"); + return NULL; + } + Py_ssize_t listSize = PyList_Size(listObj); + num_network_interface_names = (size_t)listSize; + } + struct s3_meta_request_binding *meta_request = aws_mem_calloc(allocator, 1, sizeof(struct s3_meta_request_binding)); if (!meta_request) { return PyErr_AwsLastError(); @@ -464,6 +480,15 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { } } + if (num_network_interface_names > 0) { + network_interface_names = + aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); + for (Py_ssize_t i = 0; i < listSize; ++i) { + PyObject *strObj = PyList_GetItem(listObj, i); + network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); + } + } + struct aws_s3_meta_request_options s3_meta_request_opt = { .type = type, .operation_name = aws_byte_cursor_from_c_str(operation_name), @@ -479,12 +504,15 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { .part_size = part_size, .multipart_upload_threshold = multipart_upload_threshold, .user_data = meta_request, + .network_interface_names = network_interface_names, + .num_network_interface_names = num_network_interface_names, }; if (aws_high_res_clock_get_ticks(&meta_request->last_sampled_time)) { goto error; } meta_request->native = aws_s3_client_make_meta_request(s3_client, &s3_meta_request_opt); + aws_mem_cleanup(allocator, network_interface_names); if (meta_request->native == NULL) { PyErr_SetAwsLastError(); goto error; From e21ae7954f93ff2710db70ee7ad0fa0179fa42c6 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 13:15:38 -0700 Subject: [PATCH 02/25] wip --- awscrt/s3.py | 27 ++++++++++--------- crt/aws-c-http | 2 +- crt/aws-c-io | 2 +- crt/aws-c-s3 | 2 +- source/s3_client.c | 58 +++++++++++++++++++++++++++++----------- source/s3_meta_request.c | 32 ++-------------------- 6 files changed, 62 insertions(+), 61 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index 6676fcb3d..018c76034 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -205,6 +205,9 @@ class S3Client(NativeResource): client can use for buffering data for requests. Default values scale with target throughput and are currently between 2GiB and 8GiB (may change in future) + + network_interface_names: (Optional[list(str)]) + """ __slots__ = ('shutdown_event', '_region') @@ -222,13 +225,17 @@ def __init__( multipart_upload_threshold=None, throughput_target_gbps=None, enable_s3express=False, - memory_limit=None): + memory_limit=None, + network_interface_names=None): assert isinstance(bootstrap, ClientBootstrap) or bootstrap is None assert isinstance(region, str) assert isinstance(signing_config, AwsSigningConfig) or signing_config is None assert isinstance(credential_provider, AwsCredentialsProvider) or credential_provider is None assert isinstance(tls_connection_options, TlsConnectionOptions) or tls_connection_options is None assert isinstance(part_size, int) or part_size is None + assert isinstance(network_interface_names, list) and all(isinstance(name, str) + for name in network_interface_names) or network_interface_names is None + assert isinstance( throughput_target_gbps, int) or isinstance( @@ -284,7 +291,8 @@ def on_shutdown(): throughput_target_gbps, enable_s3express, memory_limit, - s3_client_core) + s3_client_core, + network_interface_names) def make_request( self, @@ -302,8 +310,7 @@ def make_request( on_headers=None, on_body=None, on_done=None, - on_progress=None, - network_interface_names=None): + on_progress=None): """Create the Request to the the S3 server, :attr:`~S3RequestType.GET_OBJECT`/:attr:`~S3RequestType.PUT_OBJECT` requests are split it into multi-part requests under the hood for acceleration. @@ -450,8 +457,6 @@ def make_request( * `**kwargs` (dict): Forward-compatibility kwargs. - network_interface_names: (Optional[list(str)]) - Returns: S3Request """ @@ -471,8 +476,7 @@ def make_request( on_body=on_body, on_done=on_done, on_progress=on_progress, - region=self._region, - network_interface_names=network_interface_names) + region=self._region) class S3Request(NativeResource): @@ -509,8 +513,7 @@ def __init__( on_body=None, on_done=None, on_progress=None, - region=None, - network_interface_names=None): + region=None): assert isinstance(client, S3Client) assert isinstance(request, HttpRequest) assert callable(on_headers) or on_headers is None @@ -518,7 +521,6 @@ def __init__( assert callable(on_done) or on_done is None assert isinstance(part_size, int) or part_size is None assert isinstance(multipart_upload_threshold, int) or multipart_upload_threshold is None - assert isinstance(network_interface_names, list) and all(isinstance(name, str) for name in network_interface_names) or network_interface_names is None if type == S3RequestType.DEFAULT and not operation_name: raise ValueError("'operation_name' must be set when using S3RequestType.DEFAULT") @@ -570,8 +572,7 @@ def __init__( validate_response_checksum, part_size, multipart_upload_threshold, - s3_request_core, - network_interface_names) + s3_request_core) @property def finished_future(self): diff --git a/crt/aws-c-http b/crt/aws-c-http index d83f8d701..3f123bd68 160000 --- a/crt/aws-c-http +++ b/crt/aws-c-http @@ -1 +1 @@ -Subproject commit d83f8d70143ddce5ab4e479175fbd44ba994211b +Subproject commit 3f123bd68001240f64a690db93b10910f1eb7d41 diff --git a/crt/aws-c-io b/crt/aws-c-io index 878b4fa02..20eeb6b0e 160000 --- a/crt/aws-c-io +++ b/crt/aws-c-io @@ -1 +1 @@ -Subproject commit 878b4fa027bda4041493f06e0562d5e98bb3deb8 +Subproject commit 20eeb6b0e4b736dd96552e46fe397cb26c89a9c7 diff --git a/crt/aws-c-s3 b/crt/aws-c-s3 index cb431ba06..75c62cd46 160000 --- a/crt/aws-c-s3 +++ b/crt/aws-c-s3 @@ -1 +1 @@ -Subproject commit cb431ba06b5d3db4373cd8fb55d6c16464cbf2ea +Subproject commit 75c62cd469dd113cb210a8e9047e24ef2947e780 diff --git a/source/s3_client.c b/source/s3_client.c index 47d785698..8fd14323f 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -245,22 +245,24 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { struct aws_allocator *allocator = aws_py_get_allocator(); - PyObject *bootstrap_py; /* O */ - PyObject *signing_config_py; /* O */ - PyObject *credential_provider_py; /* O */ - PyObject *tls_options_py; /* O */ - PyObject *on_shutdown_py; /* O */ - struct aws_byte_cursor region; /* s# */ - int tls_mode; /* i */ - uint64_t part_size; /* K */ - uint64_t multipart_upload_threshold; /* K */ - double throughput_target_gbps; /* d */ - int enable_s3express; /* p */ - uint64_t mem_limit; /* K */ - PyObject *py_core; /* O */ + PyObject *bootstrap_py; /* O */ + PyObject *signing_config_py; /* O */ + PyObject *credential_provider_py; /* O */ + PyObject *tls_options_py; /* O */ + PyObject *on_shutdown_py; /* O */ + struct aws_byte_cursor region; /* s# */ + int tls_mode; /* i */ + uint64_t part_size; /* K */ + uint64_t multipart_upload_threshold; /* K */ + double throughput_target_gbps; /* d */ + int enable_s3express; /* p */ + uint64_t mem_limit; /* K */ + PyObject *py_core; /* O */ + PyObject *network_interface_names_py; /* O */ + if (!PyArg_ParseTuple( args, - "OOOOOs#iKKdpKO", + "OOOOOs#iKKdpKOO", &bootstrap_py, &signing_config_py, &credential_provider_py, @@ -274,7 +276,8 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { &throughput_target_gbps, &enable_s3express, &mem_limit, - &py_core)) { + &py_core, + &network_interface_names_py)) { return NULL; } @@ -319,6 +322,19 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { signing_config = &default_signing_config; } + struct aws_byte_cursor *network_interface_names = NULL; + int num_network_interface_names = 0; + + if (network_interface_names_py != Py_None) { + if (!PyList_Check(network_interface_names_py)) { + // waahm7: todo, correct way to raise errors? + PyErr_SetString(PyExc_TypeError, "Expected a list"); + return NULL; + } + Py_ssize_t listSize = PyList_Size(network_interface_names_py); + num_network_interface_names = (size_t)listSize; + } + struct s3_client_binding *s3_client = aws_mem_calloc(allocator, 1, sizeof(struct s3_client_binding)); /* From hereon, we need to clean up if errors occur */ @@ -335,6 +351,14 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { s3_client->py_core = py_core; Py_INCREF(s3_client->py_core); + if (num_network_interface_names > 0) { + network_interface_names = + aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); + for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { + PyObject *strObj = PyList_GetItem(network_interface_names_py, i); + network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); + } + } struct aws_s3_client_config s3_config = { .region = region, @@ -349,9 +373,13 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { .shutdown_callback = s_s3_client_shutdown, .shutdown_callback_user_data = s3_client, .enable_s3express = enable_s3express, + .network_interface_names_array = network_interface_names, + .num_network_interface_names = num_network_interface_names, }; s3_client->native = aws_s3_client_new(allocator, &s3_config); + aws_mem_release(allocator, network_interface_names); + if (s3_client->native == NULL) { PyErr_SetAwsLastError(); goto error; diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index c4c68b542..7586537e5 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -377,11 +377,9 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { uint64_t part_size; /* K */ uint64_t multipart_upload_threshold; /* K */ PyObject *py_core; /* O */ - PyObject *network_interface_names_py; /* O */ - if (!PyArg_ParseTuple( args, - "OOOizOOzzs#iipKKOO", + "OOOizOOzzs#iipKKO", &py_s3_request, &s3_client_py, &http_request_py, @@ -398,8 +396,7 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { &validate_response_checksum, &part_size, &multipart_upload_threshold, - &py_core, - &network_interface_names_py)) { + &py_core)) { return NULL; } struct aws_s3_client *s3_client = aws_py_get_s3_client(s3_client_py); @@ -441,19 +438,6 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { .validate_response_checksum = validate_response_checksum != 0, }; - struct aws_byte_cursor *network_interface_names = NULL; - int num_network_interface_names = 0; - - if (network_interface_names_py != Py_None) { - if (!PyList_Check(network_interface_names_py)) { - // waahm7: todo, correct way to raise errors? - PyErr_SetString(PyExc_TypeError, "Expected a list"); - return NULL; - } - Py_ssize_t listSize = PyList_Size(listObj); - num_network_interface_names = (size_t)listSize; - } - struct s3_meta_request_binding *meta_request = aws_mem_calloc(allocator, 1, sizeof(struct s3_meta_request_binding)); if (!meta_request) { return PyErr_AwsLastError(); @@ -480,15 +464,6 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { } } - if (num_network_interface_names > 0) { - network_interface_names = - aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); - for (Py_ssize_t i = 0; i < listSize; ++i) { - PyObject *strObj = PyList_GetItem(listObj, i); - network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); - } - } - struct aws_s3_meta_request_options s3_meta_request_opt = { .type = type, .operation_name = aws_byte_cursor_from_c_str(operation_name), @@ -504,15 +479,12 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) { .part_size = part_size, .multipart_upload_threshold = multipart_upload_threshold, .user_data = meta_request, - .network_interface_names = network_interface_names, - .num_network_interface_names = num_network_interface_names, }; if (aws_high_res_clock_get_ticks(&meta_request->last_sampled_time)) { goto error; } meta_request->native = aws_s3_client_make_meta_request(s3_client, &s3_meta_request_opt); - aws_mem_cleanup(allocator, network_interface_names); if (meta_request->native == NULL) { PyErr_SetAwsLastError(); goto error; From 6b0f9a672ca4147b554e1dc762c55752efb58826 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 13:17:12 -0700 Subject: [PATCH 03/25] if check before release --- source/s3_client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index 8fd14323f..77adf7921 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -378,7 +378,9 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { }; s3_client->native = aws_s3_client_new(allocator, &s3_config); - aws_mem_release(allocator, network_interface_names); + if (network_interface_names) { + aws_mem_release(allocator, network_interface_names); + } if (s3_client->native == NULL) { PyErr_SetAwsLastError(); From c32b167b954cc46e711f8294304e570e26a95e4f Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 13:29:39 -0700 Subject: [PATCH 04/25] add test --- source/s3_client.c | 4 +--- test/test_s3.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 77adf7921..8fd14323f 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -378,9 +378,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { }; s3_client->native = aws_s3_client_new(allocator, &s3_config); - if (network_interface_names) { - aws_mem_release(allocator, network_interface_names); - } + aws_mem_release(allocator, network_interface_names); if (s3_client->native == NULL) { PyErr_SetAwsLastError(); diff --git a/test/test_s3.py b/test/test_s3.py index 9d3168725..c8a048172 100644 --- a/test/test_s3.py +++ b/test/test_s3.py @@ -158,7 +158,8 @@ def s3_client_new( part_size=0, is_cancel_test=False, enable_s3express=False, - mem_limit=None): + mem_limit=None, + network_interface_names=None): if is_cancel_test: # for cancellation tests, make things slow, so it's less likely that @@ -189,7 +190,8 @@ def s3_client_new( part_size=part_size, throughput_target_gbps=throughput_target_gbps, enable_s3express=enable_s3express, - memory_limit=mem_limit) + memory_limit=mem_limit, + network_interface_names=network_interface_names) return s3_client @@ -221,6 +223,10 @@ def test_sanity_secure(self): s3_client = s3_client_new(True, self.region) self.assertIsNotNone(s3_client) + def test_sanity_network_interface_names(self): + s3_client = s3_client_new(True, self.region, network_interface_names=["eth0", "eth1"]) + self.assertIsNotNone(s3_client) + def test_wait_shutdown(self): s3_client = s3_client_new(False, self.region) self.assertIsNotNone(s3_client) From 09bce25bf983010b0661559741b9409aba331df4 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 14:14:03 -0700 Subject: [PATCH 05/25] improve error handling --- crt/aws-c-s3 | 2 +- source/s3_client.c | 24 +++++++++++++----------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/crt/aws-c-s3 b/crt/aws-c-s3 index 75c62cd46..54769f1eb 160000 --- a/crt/aws-c-s3 +++ b/crt/aws-c-s3 @@ -1 +1 @@ -Subproject commit 75c62cd469dd113cb210a8e9047e24ef2947e780 +Subproject commit 54769f1ebd835b7b35ec3bcbaa0acde3cdd7786f diff --git a/source/s3_client.c b/source/s3_client.c index 8fd14323f..8a65b9502 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -327,8 +327,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { if (network_interface_names_py != Py_None) { if (!PyList_Check(network_interface_names_py)) { - // waahm7: todo, correct way to raise errors? - PyErr_SetString(PyExc_TypeError, "Expected a list"); + PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a list"); return NULL; } Py_ssize_t listSize = PyList_Size(network_interface_names_py); @@ -338,7 +337,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { struct s3_client_binding *s3_client = aws_mem_calloc(allocator, 1, sizeof(struct s3_client_binding)); /* From hereon, we need to clean up if errors occur */ - + int result = AWS_OP_ERR; PyObject *capsule = PyCapsule_New(s3_client, s_capsule_name_s3_client, s_s3_client_capsule_destructor); if (!capsule) { aws_credentials_release(anonymous_credentials); @@ -357,6 +356,9 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { PyObject *strObj = PyList_GetItem(network_interface_names_py, i); network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); + if(network_interface_names[i].ptr == NULL) { + goto cleanup; + } } } @@ -378,17 +380,17 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { }; s3_client->native = aws_s3_client_new(allocator, &s3_config); - aws_mem_release(allocator, network_interface_names); - if (s3_client->native == NULL) { PyErr_SetAwsLastError(); - goto error; + goto cleanup; } + result = AWS_OP_SUCCESS; +cleanup: aws_credentials_release(anonymous_credentials); + aws_mem_release(allocator, network_interface_names); + if(result != AWS_OP_SUCCESS){ + Py_DECREF(capsule); + return NULL; + } return capsule; - -error: - aws_credentials_release(anonymous_credentials); - Py_DECREF(capsule); - return NULL; } From ba7de63981321a1b333942aa6716135526268da7 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 14:21:26 -0700 Subject: [PATCH 06/25] add docs --- awscrt/s3.py | 8 ++++++-- source/s3_client.c | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index 018c76034..f5148d305 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -206,8 +206,12 @@ class S3Client(NativeResource): Default values scale with target throughput and are currently between 2GiB and 8GiB (may change in future) - network_interface_names: (Optional[list(str)]) - + network_interface_names: (Optional[list(str)]) A list of network interface names. The client will distribute the + connections across network interface names provided in this array. If any interface name is invalid, goes down, + or has any issues like network access, you will see connection failures. + This option is only supported on Linux, MacOS, and platforms that have either SO_BINDTODEVICE or IP_BOUND_IF. It + is not supported on Windows. `AWS_ERROR_PLATFORM_NOT_SUPPORTED` will be raised on unsupported platforms. On + Linux, SO_BINDTODEVICE is used and requires kernel version >= 5.7 or root privileges. """ __slots__ = ('shutdown_event', '_region') diff --git a/source/s3_client.c b/source/s3_client.c index 8a65b9502..ea7a9ff02 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -338,6 +338,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { /* From hereon, we need to clean up if errors occur */ int result = AWS_OP_ERR; + PyObject *capsule = PyCapsule_New(s3_client, s_capsule_name_s3_client, s_s3_client_capsule_destructor); if (!capsule) { aws_credentials_release(anonymous_credentials); @@ -385,6 +386,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { goto cleanup; } result = AWS_OP_SUCCESS; + cleanup: aws_credentials_release(anonymous_credentials); aws_mem_release(allocator, network_interface_names); From dd1ef853358e7a2a97dd9c69a2bdceb7e5339fd3 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 14:22:33 -0700 Subject: [PATCH 07/25] update docs --- awscrt/s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index f5148d305..d268594ff 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -207,7 +207,7 @@ class S3Client(NativeResource): between 2GiB and 8GiB (may change in future) network_interface_names: (Optional[list(str)]) A list of network interface names. The client will distribute the - connections across network interface names provided in this array. If any interface name is invalid, goes down, + connections across network interfaces provided in this list. If any interface name is invalid, goes down, or has any issues like network access, you will see connection failures. This option is only supported on Linux, MacOS, and platforms that have either SO_BINDTODEVICE or IP_BOUND_IF. It is not supported on Windows. `AWS_ERROR_PLATFORM_NOT_SUPPORTED` will be raised on unsupported platforms. On From 88ce700f836c1c3f52005938028ace0658e4be72 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 8 Jul 2024 14:29:53 -0700 Subject: [PATCH 08/25] lint --- source/s3_client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index ea7a9ff02..dfbc371e2 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -357,7 +357,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { PyObject *strObj = PyList_GetItem(network_interface_names_py, i); network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); - if(network_interface_names[i].ptr == NULL) { + if (network_interface_names[i].ptr == NULL) { goto cleanup; } } @@ -390,7 +390,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { cleanup: aws_credentials_release(anonymous_credentials); aws_mem_release(allocator, network_interface_names); - if(result != AWS_OP_SUCCESS){ + if (result != AWS_OP_SUCCESS) { Py_DECREF(capsule); return NULL; } From 4c923f2313c9634f300bb7047c1c1ddd610c7c35 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:09:22 -0700 Subject: [PATCH 09/25] update logic --- awscrt/s3.py | 2 -- source/s3_client.c | 33 +++++++++++++++------------------ test/test_s3.py | 8 +++++++- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index d268594ff..a20650244 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -237,8 +237,6 @@ def __init__( assert isinstance(credential_provider, AwsCredentialsProvider) or credential_provider is None assert isinstance(tls_connection_options, TlsConnectionOptions) or tls_connection_options is None assert isinstance(part_size, int) or part_size is None - assert isinstance(network_interface_names, list) and all(isinstance(name, str) - for name in network_interface_names) or network_interface_names is None assert isinstance( throughput_target_gbps, diff --git a/source/s3_client.c b/source/s3_client.c index dfbc371e2..8708e2b7d 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -307,6 +307,9 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { struct aws_signing_config_aws *signing_config = NULL; struct aws_credentials *anonymous_credentials = NULL; + struct aws_byte_cursor *network_interface_names = NULL; + int num_network_interface_names = 0; + if (signing_config_py != Py_None) { signing_config = aws_py_get_signing_config(signing_config_py); if (!signing_config) { @@ -322,22 +325,9 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { signing_config = &default_signing_config; } - struct aws_byte_cursor *network_interface_names = NULL; - int num_network_interface_names = 0; - - if (network_interface_names_py != Py_None) { - if (!PyList_Check(network_interface_names_py)) { - PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a list"); - return NULL; - } - Py_ssize_t listSize = PyList_Size(network_interface_names_py); - num_network_interface_names = (size_t)listSize; - } - - struct s3_client_binding *s3_client = aws_mem_calloc(allocator, 1, sizeof(struct s3_client_binding)); - /* From hereon, we need to clean up if errors occur */ - int result = AWS_OP_ERR; + bool success = false; + struct s3_client_binding *s3_client = aws_mem_calloc(allocator, 1, sizeof(struct s3_client_binding)); PyObject *capsule = PyCapsule_New(s3_client, s_capsule_name_s3_client, s_s3_client_capsule_destructor); if (!capsule) { @@ -351,7 +341,14 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { s3_client->py_core = py_core; Py_INCREF(s3_client->py_core); - if (num_network_interface_names > 0) { + + if (network_interface_names_py != Py_None) { + if (!PyList_Check(network_interface_names_py)) { + PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a list"); + goto cleanup; + } + Py_ssize_t listSize = PyList_Size(network_interface_names_py); + num_network_interface_names = (size_t)listSize; network_interface_names = aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { @@ -385,12 +382,12 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { PyErr_SetAwsLastError(); goto cleanup; } - result = AWS_OP_SUCCESS; + success = true; cleanup: aws_credentials_release(anonymous_credentials); aws_mem_release(allocator, network_interface_names); - if (result != AWS_OP_SUCCESS) { + if (!success) { Py_DECREF(capsule); return NULL; } diff --git a/test/test_s3.py b/test/test_s3.py index c8a048172..bc32eee49 100644 --- a/test/test_s3.py +++ b/test/test_s3.py @@ -9,14 +9,19 @@ import math import shutil import time +from sys import stdout + from test import NativeResourceTest from concurrent.futures import Future from multiprocessing import Process from awscrt.http import HttpHeaders, HttpRequest from awscrt.s3 import S3Client, S3RequestType, create_default_s3_signing_config -from awscrt.io import ClientBootstrap, ClientTlsContext, DefaultHostResolver, EventLoopGroup, TlsConnectionOptions, TlsContextOptions +from awscrt.io import ClientBootstrap, ClientTlsContext, DefaultHostResolver, EventLoopGroup, TlsConnectionOptions, \ + TlsContextOptions, LogLevel from awscrt.auth import AwsCredentials, AwsCredentialsProvider, AwsSignatureType, AwsSignedBodyHeaderType, AwsSignedBodyValue, AwsSigningAlgorithm, AwsSigningConfig +from awscrt.io import init_logging + from awscrt.s3 import ( S3ChecksumAlgorithm, S3ChecksumConfig, @@ -224,6 +229,7 @@ def test_sanity_secure(self): self.assertIsNotNone(s3_client) def test_sanity_network_interface_names(self): + init_logging(LogLevel.Debug, "stdout") s3_client = s3_client_new(True, self.region, network_interface_names=["eth0", "eth1"]) self.assertIsNotNone(s3_client) From 7d85fc1c1f836ba471e3c7e4d122a77dd6d9cb8a Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:17:39 -0700 Subject: [PATCH 10/25] better error handling --- source/s3_client.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 8708e2b7d..0301f79a9 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -309,11 +309,14 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { struct aws_credentials *anonymous_credentials = NULL; struct aws_byte_cursor *network_interface_names = NULL; int num_network_interface_names = 0; + PyObject *capsule = NULL; + /* From hereon, we need to clean up if errors occur */ + bool success = false; if (signing_config_py != Py_None) { signing_config = aws_py_get_signing_config(signing_config_py); if (!signing_config) { - return NULL; + goto cleanup; } } else if (credential_provider) { aws_s3_init_default_signing_config(&default_signing_config, region, credential_provider); @@ -325,15 +328,11 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { signing_config = &default_signing_config; } - /* From hereon, we need to clean up if errors occur */ - bool success = false; struct s3_client_binding *s3_client = aws_mem_calloc(allocator, 1, sizeof(struct s3_client_binding)); - PyObject *capsule = PyCapsule_New(s3_client, s_capsule_name_s3_client, s_s3_client_capsule_destructor); + capsule = PyCapsule_New(s3_client, s_capsule_name_s3_client, s_s3_client_capsule_destructor); if (!capsule) { - aws_credentials_release(anonymous_credentials); - aws_mem_release(allocator, s3_client); - return NULL; + goto cleanup; } s3_client->on_shutdown = on_shutdown_py; @@ -388,7 +387,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { aws_credentials_release(anonymous_credentials); aws_mem_release(allocator, network_interface_names); if (!success) { - Py_DECREF(capsule); + Py_XDECREF(capsule); return NULL; } return capsule; From ced3fd0fb6b184aec4b16f01314f786a6b8cf4f8 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:18:49 -0700 Subject: [PATCH 11/25] s3 client mem release --- source/s3_client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/s3_client.c b/source/s3_client.c index 0301f79a9..14b6183c5 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -332,6 +332,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { capsule = PyCapsule_New(s3_client, s_capsule_name_s3_client, s_s3_client_capsule_destructor); if (!capsule) { + aws_mem_release(allocator, s3_client); goto cleanup; } From bf48f382afb1bfdf589ef5f61da87554c1615b9d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:22:49 -0700 Subject: [PATCH 12/25] move pycore back to end --- awscrt/s3.py | 4 ++-- source/s3_client.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index a20650244..5a805155e 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -293,8 +293,8 @@ def on_shutdown(): throughput_target_gbps, enable_s3express, memory_limit, - s3_client_core, - network_interface_names) + network_interface_names, + s3_client_core) def make_request( self, diff --git a/source/s3_client.c b/source/s3_client.c index 14b6183c5..2b0180fc7 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -257,8 +257,8 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { double throughput_target_gbps; /* d */ int enable_s3express; /* p */ uint64_t mem_limit; /* K */ - PyObject *py_core; /* O */ PyObject *network_interface_names_py; /* O */ + PyObject *py_core; /* O */ if (!PyArg_ParseTuple( args, @@ -276,8 +276,8 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { &throughput_target_gbps, &enable_s3express, &mem_limit, - &py_core, - &network_interface_names_py)) { + &network_interface_names_py, + &py_core)) { return NULL; } From 36af1461a92141e3cbead6508f6ef43b60407f45 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:33:46 -0700 Subject: [PATCH 13/25] use the pysequence API --- source/s3_client.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 2b0180fc7..e16f57c0a 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -343,18 +343,23 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { Py_INCREF(s3_client->py_core); if (network_interface_names_py != Py_None) { - if (!PyList_Check(network_interface_names_py)) { - PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a list"); + if (!PySequence_Check(network_interface_names_py)) { + PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a sequence"); goto cleanup; } - Py_ssize_t listSize = PyList_Size(network_interface_names_py); + Py_ssize_t listSize = PySequence_Size(network_interface_names_py); num_network_interface_names = (size_t)listSize; network_interface_names = aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { - PyObject *strObj = PyList_GetItem(network_interface_names_py, i); + PyObject *strObj = PySequence_GetItem(network_interface_names_py, i); + if(!strObj) { + PyErr_SetString(PyExc_TypeError, "Expected network_interface_names elements to be non-null"); + goto cleanup; + } network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); if (network_interface_names[i].ptr == NULL) { + PyErr_SetString(PyExc_TypeError, "Expected all network_interface_names elements to be string"); goto cleanup; } } From 89c61e0042cae82fb913c9696e0aba4b7a950877 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:37:56 -0700 Subject: [PATCH 14/25] update docs --- awscrt/s3.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index 5a805155e..a740cb42a 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -206,8 +206,8 @@ class S3Client(NativeResource): Default values scale with target throughput and are currently between 2GiB and 8GiB (may change in future) - network_interface_names: (Optional[list(str)]) A list of network interface names. The client will distribute the - connections across network interfaces provided in this list. If any interface name is invalid, goes down, + network_interface_names: (Optional[Sequence(str)]) A sequence of network interface names. The client will distribute the + connections across network interfaces provided. If any interface name is invalid, goes down, or has any issues like network access, you will see connection failures. This option is only supported on Linux, MacOS, and platforms that have either SO_BINDTODEVICE or IP_BOUND_IF. It is not supported on Windows. `AWS_ERROR_PLATFORM_NOT_SUPPORTED` will be raised on unsupported platforms. On @@ -237,7 +237,6 @@ def __init__( assert isinstance(credential_provider, AwsCredentialsProvider) or credential_provider is None assert isinstance(tls_connection_options, TlsConnectionOptions) or tls_connection_options is None assert isinstance(part_size, int) or part_size is None - assert isinstance( throughput_target_gbps, int) or isinstance( From 48fc70ed4b30d626786b493dbca123f16f714ab6 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:43:29 -0700 Subject: [PATCH 15/25] cleanup --- awscrt/s3.py | 2 +- source/s3_client.c | 12 ++++++------ test/test_s3.py | 5 +---- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index a740cb42a..78803f173 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -207,7 +207,7 @@ class S3Client(NativeResource): between 2GiB and 8GiB (may change in future) network_interface_names: (Optional[Sequence(str)]) A sequence of network interface names. The client will distribute the - connections across network interfaces provided. If any interface name is invalid, goes down, + connections across network interfaces. If any interface name is invalid, goes down, or has any issues like network access, you will see connection failures. This option is only supported on Linux, MacOS, and platforms that have either SO_BINDTODEVICE or IP_BOUND_IF. It is not supported on Windows. `AWS_ERROR_PLATFORM_NOT_SUPPORTED` will be raised on unsupported platforms. On diff --git a/source/s3_client.c b/source/s3_client.c index e16f57c0a..a63f9c8c3 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -309,14 +309,14 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { struct aws_credentials *anonymous_credentials = NULL; struct aws_byte_cursor *network_interface_names = NULL; int num_network_interface_names = 0; - PyObject *capsule = NULL; + PyObject *capsule = NULL; /* From hereon, we need to clean up if errors occur */ bool success = false; if (signing_config_py != Py_None) { signing_config = aws_py_get_signing_config(signing_config_py); if (!signing_config) { - goto cleanup; + goto cleanup; } } else if (credential_provider) { aws_s3_init_default_signing_config(&default_signing_config, region, credential_provider); @@ -344,7 +344,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { if (network_interface_names_py != Py_None) { if (!PySequence_Check(network_interface_names_py)) { - PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a sequence"); + PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a sequence."); goto cleanup; } Py_ssize_t listSize = PySequence_Size(network_interface_names_py); @@ -353,13 +353,13 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { PyObject *strObj = PySequence_GetItem(network_interface_names_py, i); - if(!strObj) { - PyErr_SetString(PyExc_TypeError, "Expected network_interface_names elements to be non-null"); + if (!strObj) { + PyErr_SetString(PyExc_TypeError, "Expected network_interface_names elements to be non-null."); goto cleanup; } network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); if (network_interface_names[i].ptr == NULL) { - PyErr_SetString(PyExc_TypeError, "Expected all network_interface_names elements to be string"); + PyErr_SetString(PyExc_TypeError, "Expected all network_interface_names elements to be strings."); goto cleanup; } } diff --git a/test/test_s3.py b/test/test_s3.py index bc32eee49..c475a1a50 100644 --- a/test/test_s3.py +++ b/test/test_s3.py @@ -9,7 +9,6 @@ import math import shutil import time -from sys import stdout from test import NativeResourceTest from concurrent.futures import Future @@ -18,9 +17,8 @@ from awscrt.http import HttpHeaders, HttpRequest from awscrt.s3 import S3Client, S3RequestType, create_default_s3_signing_config from awscrt.io import ClientBootstrap, ClientTlsContext, DefaultHostResolver, EventLoopGroup, TlsConnectionOptions, \ - TlsContextOptions, LogLevel + TlsContextOptions from awscrt.auth import AwsCredentials, AwsCredentialsProvider, AwsSignatureType, AwsSignedBodyHeaderType, AwsSignedBodyValue, AwsSigningAlgorithm, AwsSigningConfig -from awscrt.io import init_logging from awscrt.s3 import ( S3ChecksumAlgorithm, @@ -229,7 +227,6 @@ def test_sanity_secure(self): self.assertIsNotNone(s3_client) def test_sanity_network_interface_names(self): - init_logging(LogLevel.Debug, "stdout") s3_client = s3_client_new(True, self.region, network_interface_names=["eth0", "eth1"]) self.assertIsNotNone(s3_client) From d72a5eaf04cc0706e5b7e0f42646b518d6575cc1 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 09:45:13 -0700 Subject: [PATCH 16/25] more cleanup --- test/test_s3.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/test_s3.py b/test/test_s3.py index c475a1a50..f7b15fdda 100644 --- a/test/test_s3.py +++ b/test/test_s3.py @@ -9,15 +9,13 @@ import math import shutil import time - from test import NativeResourceTest from concurrent.futures import Future from multiprocessing import Process from awscrt.http import HttpHeaders, HttpRequest from awscrt.s3 import S3Client, S3RequestType, create_default_s3_signing_config -from awscrt.io import ClientBootstrap, ClientTlsContext, DefaultHostResolver, EventLoopGroup, TlsConnectionOptions, \ - TlsContextOptions +from awscrt.io import ClientBootstrap, ClientTlsContext, DefaultHostResolver, EventLoopGroup, TlsConnectionOptions, TlsContextOptions from awscrt.auth import AwsCredentials, AwsCredentialsProvider, AwsSignatureType, AwsSignedBodyHeaderType, AwsSignedBodyValue, AwsSigningAlgorithm, AwsSigningConfig from awscrt.s3 import ( From fc5ffb804d8ca655d66d7556206f73916108bd4e Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 10:02:42 -0700 Subject: [PATCH 17/25] cleanup --- test/test_s3.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_s3.py b/test/test_s3.py index f7b15fdda..c8a048172 100644 --- a/test/test_s3.py +++ b/test/test_s3.py @@ -17,7 +17,6 @@ from awscrt.s3 import S3Client, S3RequestType, create_default_s3_signing_config from awscrt.io import ClientBootstrap, ClientTlsContext, DefaultHostResolver, EventLoopGroup, TlsConnectionOptions, TlsContextOptions from awscrt.auth import AwsCredentials, AwsCredentialsProvider, AwsSignatureType, AwsSignedBodyHeaderType, AwsSignedBodyValue, AwsSigningAlgorithm, AwsSigningConfig - from awscrt.s3 import ( S3ChecksumAlgorithm, S3ChecksumConfig, From 0ae4e04c5e0b7c563f2783355f1d0416d9a6df3b Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 14:32:24 -0700 Subject: [PATCH 18/25] Update source/s3_client.c Co-authored-by: Michael Graeb --- source/s3_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index a63f9c8c3..7e669d490 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -308,7 +308,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { struct aws_signing_config_aws *signing_config = NULL; struct aws_credentials *anonymous_credentials = NULL; struct aws_byte_cursor *network_interface_names = NULL; - int num_network_interface_names = 0; + size_t num_network_interface_names = 0; PyObject *capsule = NULL; /* From hereon, we need to clean up if errors occur */ bool success = false; From 503b93827f57e9a99fbe72fb1ef2d5b5bd9290d0 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 14:46:21 -0700 Subject: [PATCH 19/25] error checking and decref for get item. --- source/s3_client.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 7e669d490..814dd94a1 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -347,17 +347,21 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a sequence."); goto cleanup; } - Py_ssize_t listSize = PySequence_Size(network_interface_names_py); - num_network_interface_names = (size_t)listSize; + Py_ssize_t list_size = PySequence_Size(network_interface_names_py); + if (list_size < 0) { + goto cleanup; + } + num_network_interface_names = (size_t)list_size; network_interface_names = aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { - PyObject *strObj = PySequence_GetItem(network_interface_names_py, i); - if (!strObj) { + PyObject *str_obj = PySequence_GetItem(network_interface_names_py, i); /* New reference */ + if (!str_obj) { PyErr_SetString(PyExc_TypeError, "Expected network_interface_names elements to be non-null."); goto cleanup; } - network_interface_names[i] = aws_byte_cursor_from_pyunicode(strObj); + network_interface_names[i] = aws_byte_cursor_from_pyunicode(str_obj); + Py_DECREF(str_obj); if (network_interface_names[i].ptr == NULL) { PyErr_SetString(PyExc_TypeError, "Expected all network_interface_names elements to be strings."); goto cleanup; From 1d22f3d9c4f4bb2f51d6ed24101b626f9c7a6588 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 14:51:20 -0700 Subject: [PATCH 20/25] fix loop --- source/s3_client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 814dd94a1..565a808d7 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -351,10 +351,10 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { if (list_size < 0) { goto cleanup; } - num_network_interface_names = (size_t)list_size; + num_network_interface_names = (size_t) list_size; network_interface_names = aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); - for (Py_ssize_t i = 0; i < num_network_interface_names; ++i) { + for (size_t i = 0; i < num_network_interface_names; ++i) { PyObject *str_obj = PySequence_GetItem(network_interface_names_py, i); /* New reference */ if (!str_obj) { PyErr_SetString(PyExc_TypeError, "Expected network_interface_names elements to be non-null."); From 93fd4df6af887dada1087a0a567985194facbe0f Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Tue, 9 Jul 2024 14:58:00 -0700 Subject: [PATCH 21/25] lint --- source/s3_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index 565a808d7..458255fcc 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -351,7 +351,7 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { if (list_size < 0) { goto cleanup; } - num_network_interface_names = (size_t) list_size; + num_network_interface_names = (size_t)list_size; network_interface_names = aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); for (size_t i = 0; i < num_network_interface_names; ++i) { From 461d5e06656495f27671a26d6888aeb79f602a72 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Wed, 10 Jul 2024 08:26:30 -0700 Subject: [PATCH 22/25] Update source/s3_client.c Co-authored-by: Michael Graeb --- source/s3_client.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index 458255fcc..fc03668d2 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -357,7 +357,6 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { for (size_t i = 0; i < num_network_interface_names; ++i) { PyObject *str_obj = PySequence_GetItem(network_interface_names_py, i); /* New reference */ if (!str_obj) { - PyErr_SetString(PyExc_TypeError, "Expected network_interface_names elements to be non-null."); goto cleanup; } network_interface_names[i] = aws_byte_cursor_from_pyunicode(str_obj); From 7cf911094558c516af363cfe4bb6674d8c621834 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Wed, 10 Jul 2024 11:05:35 -0700 Subject: [PATCH 23/25] move back to list api --- awscrt/s3.py | 11 +++++++---- source/s3_client.c | 7 +++---- test/test_s3.py | 6 ++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/awscrt/s3.py b/awscrt/s3.py index 78803f173..e8409d706 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -10,13 +10,12 @@ from awscrt import NativeResource from awscrt.http import HttpRequest from awscrt.io import ClientBootstrap, TlsConnectionOptions -from awscrt.auth import AwsCredentials, AwsCredentialsProvider, AwsSignatureType, AwsSignedBodyHeaderType, AwsSignedBodyValue, AwsSigningAlgorithm, AwsSigningConfig from awscrt.auth import AwsCredentialsProvider, AwsSignatureType, AwsSignedBodyHeaderType, AwsSignedBodyValue, \ AwsSigningAlgorithm, AwsSigningConfig import awscrt.exceptions import threading from dataclasses import dataclass -from typing import List, Optional, Tuple +from typing import List, Optional, Tuple, Sequence from enum import IntEnum @@ -230,7 +229,7 @@ def __init__( throughput_target_gbps=None, enable_s3express=False, memory_limit=None, - network_interface_names=None): + network_interface_names: Optional[Sequence[str]] = None): assert isinstance(bootstrap, ClientBootstrap) or bootstrap is None assert isinstance(region, str) assert isinstance(signing_config, AwsSigningConfig) or signing_config is None @@ -243,7 +242,7 @@ def __init__( throughput_target_gbps, float) or throughput_target_gbps is None assert isinstance(enable_s3express, bool) or enable_s3express is None - + assert isinstance(network_interface_names, Sequence) or network_interface_names is None if credential_provider and signing_config: raise ValueError("'credential_provider' has been deprecated in favor of 'signing_config'. " "Both parameters may not be set.") @@ -278,6 +277,10 @@ def on_shutdown(): throughput_target_gbps = 0 if memory_limit is None: memory_limit = 0 + if network_interface_names is not None: + # ensure this is a list, so it's simpler to process in C + if not isinstance(network_interface_names, list): + network_interface_names = list(network_interface_names) self._binding = _awscrt.s3_client_new( bootstrap, diff --git a/source/s3_client.c b/source/s3_client.c index fc03668d2..8eafd03b5 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -343,11 +343,11 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { Py_INCREF(s3_client->py_core); if (network_interface_names_py != Py_None) { - if (!PySequence_Check(network_interface_names_py)) { + if (!PyList_Check(network_interface_names_py)) { PyErr_SetString(PyExc_TypeError, "Expected network_interface_names to be a sequence."); goto cleanup; } - Py_ssize_t list_size = PySequence_Size(network_interface_names_py); + Py_ssize_t list_size = PyList_Size(network_interface_names_py); if (list_size < 0) { goto cleanup; } @@ -355,12 +355,11 @@ PyObject *aws_py_s3_client_new(PyObject *self, PyObject *args) { network_interface_names = aws_mem_calloc(allocator, num_network_interface_names, sizeof(struct aws_byte_cursor)); for (size_t i = 0; i < num_network_interface_names; ++i) { - PyObject *str_obj = PySequence_GetItem(network_interface_names_py, i); /* New reference */ + PyObject *str_obj = PyList_GetItem(network_interface_names_py, i); /* Borrowed reference */ if (!str_obj) { goto cleanup; } network_interface_names[i] = aws_byte_cursor_from_pyunicode(str_obj); - Py_DECREF(str_obj); if (network_interface_names[i].ptr == NULL) { PyErr_SetString(PyExc_TypeError, "Expected all network_interface_names elements to be strings."); goto cleanup; diff --git a/test/test_s3.py b/test/test_s3.py index c8a048172..ddde60aaa 100644 --- a/test/test_s3.py +++ b/test/test_s3.py @@ -14,9 +14,7 @@ from multiprocessing import Process from awscrt.http import HttpHeaders, HttpRequest -from awscrt.s3 import S3Client, S3RequestType, create_default_s3_signing_config -from awscrt.io import ClientBootstrap, ClientTlsContext, DefaultHostResolver, EventLoopGroup, TlsConnectionOptions, TlsContextOptions -from awscrt.auth import AwsCredentials, AwsCredentialsProvider, AwsSignatureType, AwsSignedBodyHeaderType, AwsSignedBodyValue, AwsSigningAlgorithm, AwsSigningConfig +from awscrt.auth import AwsCredentials from awscrt.s3 import ( S3ChecksumAlgorithm, S3ChecksumConfig, @@ -224,7 +222,7 @@ def test_sanity_secure(self): self.assertIsNotNone(s3_client) def test_sanity_network_interface_names(self): - s3_client = s3_client_new(True, self.region, network_interface_names=["eth0", "eth1"]) + s3_client = s3_client_new(True, self.region, network_interface_names=("eth0", "eth1")) self.assertIsNotNone(s3_client) def test_wait_shutdown(self): From a0825e090da67d5a457432b57ae50d6cf228281d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Wed, 10 Jul 2024 11:06:43 -0700 Subject: [PATCH 24/25] fix blank line --- awscrt/s3.py | 1 + 1 file changed, 1 insertion(+) diff --git a/awscrt/s3.py b/awscrt/s3.py index e8409d706..a623cdb42 100644 --- a/awscrt/s3.py +++ b/awscrt/s3.py @@ -243,6 +243,7 @@ def __init__( float) or throughput_target_gbps is None assert isinstance(enable_s3express, bool) or enable_s3express is None assert isinstance(network_interface_names, Sequence) or network_interface_names is None + if credential_provider and signing_config: raise ValueError("'credential_provider' has been deprecated in favor of 'signing_config'. " "Both parameters may not be set.") From cab8e12619d1804641c25691b60f1d68a0930f8b Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 11 Jul 2024 14:23:30 -0700 Subject: [PATCH 25/25] latest submodules --- crt/aws-c-http | 2 +- crt/aws-c-io | 2 +- crt/aws-c-s3 | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crt/aws-c-http b/crt/aws-c-http index 3f123bd68..652e2febf 160000 --- a/crt/aws-c-http +++ b/crt/aws-c-http @@ -1 +1 @@ -Subproject commit 3f123bd68001240f64a690db93b10910f1eb7d41 +Subproject commit 652e2febf2242d6b3562267dc0dd982375ed698e diff --git a/crt/aws-c-io b/crt/aws-c-io index 20eeb6b0e..d04508d11 160000 --- a/crt/aws-c-io +++ b/crt/aws-c-io @@ -1 +1 @@ -Subproject commit 20eeb6b0e4b736dd96552e46fe397cb26c89a9c7 +Subproject commit d04508d113851f1bc15630d93490b2aa09676137 diff --git a/crt/aws-c-s3 b/crt/aws-c-s3 index 54769f1eb..94edd4747 160000 --- a/crt/aws-c-s3 +++ b/crt/aws-c-s3 @@ -1 +1 @@ -Subproject commit 54769f1ebd835b7b35ec3bcbaa0acde3cdd7786f +Subproject commit 94edd4747923f3c255c2e1b1766ee9de20a1caf8