-
Notifications
You must be signed in to change notification settings - Fork 13
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
PWV HTTP Requests Error #563
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgive the random review but this caught my eye! Couple other comments:
-
The JSONDecodeError you're seeing is because the server is working (it sends a json-like structure) but is sending "json" in a (non-)standard that the requests library doesn't like. Specifically it's representing NaN values as "nan". Probably you want to ignore those data, anyway, so it's fine to just catch the error like you're doing.
-
Later in the function, you have this:
session.data = {"timestamp": last_timestamp,
"pwv": {}}
session.data['pwv'] = last_pwv
That's a bit of an odd construction, to have pwv sometimes be an empty dict and sometimes be a float. It could cause trouble for anyone trying to use the session.data. Can you change it to simply:
session.data = {"timestamp": last_timestamp,
"pwv": last_pwv}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @mhasself. I just wanted to mention what I think is going on on the backend here. Looking at the most recent two crashes, this happens at the first query after 00:00 -- 2023-11-05T00:00:08+0000
and 2023-11-07T00:00:10+0000
are the two that I'm seeing.
The server that this agent queries takes the median of the last four data points, and I expect that code is running into some error when there are less than four points in the file its reading, so we end up with a nan
response.
thank you both for the input and catching my half-brained errors! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fixes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the logs it looks like the except
statement needs to be handling the JSONDecodeError
, not a ValueError
. Where was the ValueError
coming up?
* add try/except catch for failed http req's * fix typo in log.info message * remove try/except in wrong part of loop * fix pass error to continue * remove info message * update session.data to be more clean
Description
Addressing this error for the PWV agent:
Unable to format event {'log_logger': <Logger 'ocs.ocs_agent.OCSAgent'>, 'log_level': <LogLevel=info>, 'log_namespace': 'ocs.ocs_agent. OCSAgent', '10g_SOle': None, 'log_format': 'acq:0 CRASH: [Failure instance: Traceback: <class \'requests .exceptions .JSONDecodeError\'>: [Errno Expecting value] {"pwv": nan, "timestamp": nan}:
Motivation and Context
I'm 10% unsure that this is truly an HTTP requests failure because I don't know if I should expect/assume a 204 error to output
nan
's. Implementing a try/except to loop through and make the http request again. Added aself.log.info
message to query the requests status code the next time this happens in the agent to know more.How Has This Been Tested?
Hasn't been tested on the flask server at the site itself.
Types of changes
Checklist: