CBOR bindings #559
CBOR bindings #559
Conversation
| return _awscrt.cbor_encoder_get_encoded_data(self._binding) | ||
|
|
||
| def write_int(self, val: int): | ||
| # TODO: maybe not support bignum for now. Not needed? |
There was a problem hiding this comment.
smithy currently does not support anything above 64bits afaik so we can probably skip bignum for now
| # For negative value, the value to encode is -1 - val. | ||
| val_to_encode = -1 - val | ||
| bit_len = val_to_encode.bit_length() | ||
| if bit_len > 64: |
There was a problem hiding this comment.
wouldnt python need 65 bits to represent - u64_max? so this logic might be a bit off, since we dont need bignum for - u64_max
| Args: | ||
| val (int): value to be encoded and written to the encoded data. | ||
| """ | ||
| assert isinstance(val, int) |
There was a problem hiding this comment.
for my education, havent we already indicated in function signature htat val is an int? why do we have to check twice
|
|
||
| def write_array_start(self, number_entries: int): | ||
| """Add a start of array element. | ||
| A legistic with the `number_entries` |
| Args: | ||
| number_entries (int): number of entries in the array to be written | ||
| """ | ||
| if number_entries < 0 or number_entries > 2**64: |
There was a problem hiding this comment.
ugh, cbor spec allows 2^64 entries? not sure we would be able to allocate cbor chunk for that even if all entries are 1 byte. should we consider a lower bound
|
|
||
| PyObject *aws_py_cbor_encoder_write_unsigned_int(PyObject *self, PyObject *args) { | ||
| PyObject *pylong; | ||
| S_ENCODER_METHOD_START("O", &pylong); |
There was a problem hiding this comment.
the only thing that changes is the last part of s_cbor_encoder_write_pyobject_as? maybe more more of that into macro and let macro just call correct s_cbor_encoder_write_pyobject_as?
having a macro thats just a partial method kinda scares me
| for (Py_ssize_t i = 0; i < size; i++) { | ||
| PyObject *item = PyList_GetItem(py_list, i); | ||
| if (!item) { | ||
| return NULL; |
There was a problem hiding this comment.
so we leave it in broken state if element is not valid?
| return py_capsule; | ||
| } | ||
|
|
||
| #define S_GET_DECODER() \ |
There was a problem hiding this comment.
nit: but im not a big fan of macros inserting random code into functions, can we make macros more like a generic function that returns a value rathere then relying on side effects
| return PyErr_AwsLastError(); | ||
| } | ||
| /* TODO: implement those tags */ | ||
| switch (out_tag_val) { |
There was a problem hiding this comment.
so we currently support no tags? should we be more explcit in doc on what function supports?
DmitriyMusatkin
dismissed
their stale review
accidental approve. comments still need to be discussed
Issue #, if available:
Description of changes:
TODO:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.