Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Humanization of ints fails for values > 2,592,000 (1 month) #1358

Closed
MagikEh opened this issue Dec 22, 2024 · 3 comments · Fixed by #1359
Closed

Humanization of ints fails for values > 2,592,000 (1 month) #1358

MagikEh opened this issue Dec 22, 2024 · 3 comments · Fixed by #1359
Assignees
Labels
work-around a work-around is provided, mitigating the issue.

Comments

@MagikEh
Copy link
Contributor

MagikEh commented Dec 22, 2024

first_part= humanize.naturaldelta(d).replace("minutes","m").replace("seconds","s").replace("hours","h").replace("days","d").replace("an hour","1h").replace("a day","1d").replace("a minute","1m").replace(" ","")
shows that there were some regexes done without consideration for extreme cases.

As Such when calling durationToString(2681007.932507038), first_part becomes a month and then amonth due to the final .replace(" ","") and clogs up any sr3 status run because of the d-int(first_part[0:-1]) receiving non-digit characters.

Not sure if we want to be more intelligent with the order of operations

first_part= humanize.naturaldelta(d).replace("^an* ","1").replace("minutes*","m").replace("seconds*","s").replace("hours*","h").replace("days*","d").replace(" ","")

and then replace the rest of the array logic with more regex oriented replacements to strip out the numerical values? (to prevent string format assumptions from breaking things again)
Or if some other solution should be discovered.

  • Maybe humanize has a nice, short output like we're targeting?
  • Maybe we should just do some maths?

Fixed in operations temporarily by surrounding with a try/catch and setting the catch to output `'>30d'.
image

Here's the stacktrace that lead me to this error.

Traceback (most recent call last):
  File "/usr/bin/sr3", line 11, in <module>
    load_entry_point('metpx-sr3==3.0.56', 'console_scripts', 'sr3')()
  File "/usr/lib/python3/dist-packages/sarracenia/sr.py", line 3136, in main
    gs = sr_GlobalState(cfg, cfg.configurations)
  File "/usr/lib/python3/dist-packages/sarracenia/sr.py", line 1336, in __init__
    self._resolve()
  File "/usr/lib/python3/dist-packages/sarracenia/sr.py", line 997, in _resolve
    m['latestTransfer'] = durationToString(now - m['messageLast'])
  File "/usr/lib/python3/dist-packages/sarracenia/__init__.py", line 332, in durationToString
    rem=int(( d-int(first_part[0:-1])*60*60 ) / 60 )
ValueError: invalid literal for int() with base 10: 'amont'
@MagikEh MagikEh changed the title Humanize ints fails for more than 2,592,000 Humanization of ints fails for values > 2,592,000 (1 month) Dec 22, 2024
@petersilva petersilva self-assigned this Dec 23, 2024
@petersilva
Copy link
Contributor

so added a bunch of tests to the unit tests and found out weird stuff:

  • there are no weeks ever returned by humanize module... so we can't get that back with the current implementation.
  • instead it only changes units in months ... but... um... how big a month? 28 days? 29? 30? 31? when I put in 30 days... the module gives "30d" when I put in 31d it returns "a month" ...
fractal% python3
Python 3.12.3 (main, Nov  6 2024, 18:32:19) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from humanize import naturaldelta as nd
>>> nd( 30*24*3600 )
'30 days'
>>> nd( 31*24*3600 )
'a month'
>>> nd( 61*24*3600 )
'2 months'
>>> nd( 60*24*3600 )
'a month'
>>> 
  • after some experiments... I figured a month has 30.7 days... that seemed to give the right answer with different numbers of month (less than 1 y)
  • similarly... how long is a year? I decided it should be 365.25 days.

@petersilva
Copy link
Contributor

I like weeks... so I kept it in the conversion from a string to a second interval... but since humanize doesn't return it... it's one way.

@petersilva
Copy link
Contributor

work-around: people are removing metrics files to prevent the occurrence on other systems.

@petersilva petersilva added the work-around a work-around is provided, mitigating the issue. label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-around a work-around is provided, mitigating the issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants