Use function for datetime in CommandCompleted and throw error when testing #7217
Conversation
| # If we're testing throw an error so that we notice failures | ||
| if "pytest" in sys.modules: | ||
| raise Exception(error_msg) | ||
| else: | ||
| fire_event(Note(msg=error_msg)) | ||
| self.pb_msg = msg_cls() |
There was a problem hiding this comment.
does this also mean that "fire a warning instead of raising an exception on event serialization errors" was effectively resolved by the proto changes?
There was a problem hiding this comment.
Well, probably, but mainly because the google protobuf messages are more strongly typed, so that errors are caught here instead of on serialization. As a belt-and-suspenders thing it's probably not a bad idea to catch on serialization too (though I haven't actually seen any errors there).
| self.pb_msg = msg_cls() | ||
| error_msg = f"[{class_name}]: Unable to parse dict {kwargs}" | ||
| # If we're testing throw an error so that we notice failures | ||
| if "pytest" in sys.modules: |
There was a problem hiding this comment.
We can just error on this warning in Pytest instead of having branching logic.
https://docs.pytest.org/en/latest/how-to/capture-warnings.html
There was a problem hiding this comment.
I don't think that we want to issue warnings when users are running dbt though.
There was a problem hiding this comment.
Also I think most people are in the habit of just ignoring those pytest warnings, and we certainly aren't going to see them in Ci runs, so we still wouldn't necessarily notice that we have a logging event with invalid types.
There was a problem hiding this comment.
I don't think that we want to issue warnings when users are running dbt though.
This doesn't affect users unless they're running Pytest.
we certainly aren't going to see them in Ci runs
If you promote a warning to an error in Pytest, people will notice :)
Description
Use utility function "get_json_string_utcnow" to set the time in the CommandCompleted message. Throw errors when testing and unable to parse the kwarg dictionary in the logging event base class.
Checklist
changie newto create a changelog entry