Upgrade Tron to Python 3.10 - TRON-2435 #1071
Conversation
EmanElsaban
force-pushed
the
u/emanelsabban/TRON-2435
branch
from
7de4c89 to
25be067
Compare
EmanElsaban
requested review from
EvanKrall,
nemacysts,
jfongatyelp,
cuza and
KaspariK
| "cleanup_run": None, | ||
| "manual": False, | ||
| "large_data": [i for i in range(1_000_000)], | ||
| "large_data": [i for i in range(10000)], |
There was a problem hiding this comment.
was this meant to be changed?
There was a problem hiding this comment.
yea, I was getting this error
ClientError('An error occurred (ValidationException) when calling the TransactWriteItems operation: Item size has exceeded the maximum allowed size')
moto's mock enforces a 400kb item size limit and the 1000000 serializes to a json object greater than 400kb. I think older versions of moto didn't enforce this size limit and now that we upgraded moto it enforces it. So I reduced the number of integers we create here to be below that limit
There was a problem hiding this comment.
gotcha! i assume that since the tests are still passing, then we're still ending up splitting this object
There was a problem hiding this comment.
I suspect that by reducing the data to 10,000 integers, we're now creating an object that is smaller than our OBJECT_SIZE of 200KB. This would mean it no longer gets partitioned, and these tests are no longer verifying the multi-partition logic they were designed for.
Maybe we should also add an assert num_partitions(key) > 1 in these tests to guarantee we're actually testing the partitioning logic?
Also, outside the bounds of this PR, it might be a good time to rename these tests to something like test_save_object_requiring_partitioning. more_than_4KB is technically correct but confusing 😄
There was a problem hiding this comment.
iirc from some manual tests, 50,000 gets us ~2 partitions
There was a problem hiding this comment.
I think what might be happening here is related to OBJECT_SIZE. We don't leave a lot of overhead with our current limit. You could try changing OBJECT_SIZE to 150,000. Keep in mind that we still want to make sure we're setting large object in a way that both the pickle and json are getting partitioned.
| return source.split(",") | ||
|
|
||
| def parse(self, source: str) -> Optional[Union[List[int], List[Union[int, str]]]]: | ||
| def parse(self, source: str) -> list[int] | list[int | str] | None: |
There was a problem hiding this comment.
i have no idea why i initially wrote this type this way rather than list[int | str] | None
we can fix this in another PR though and leave this one a pretty mechanical transformation to this form of typing tho
There was a problem hiding this comment.
okay will leave it as it is now then
This PR upgrades Tron to Python 3.10. Tests are passing locally.