Donniedarko45 commented on PR #76203: URL: https://github.com/apache/airflow/pull/76203#issuecomment-6808101774
I ran into the same crash and opened #76915 before I found this PR, so +1 for getting this in. I read through the diff (I have not run it locally) and it looks right to me. A few small things: 1. `_MEMORY_LIMIT_PATTERN` needs digits before the decimal point, so `.5G`, `2.G` and `1e3m` are now rejected, while the old `float()` parsing accepted them. Probably rare in real specs, but something like `(?P<value>\d+\.?\d*|\.\d+)` would keep the first two working. 2. `M` is converted the same as `Mi`. That matches how `G` and `Gi` were already handled, but it may be worth saying so in the description or the docstring, since strictly they differ by about 5%. 3. Nit: the coding standards ask for verb names for functions, so `_convert_memory_limit_to_mebibytes` rather than `_memory_limit_to_mebibytes`. As far as I can tell, values that worked before (`512m`, `2G`, `2Gi`, `1.5G`) give the same result, and the new tests cover both error branches. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
