Several oonimeasurements fixes #1012
Conversation
LDiazN
changed the title
|
|
||
| @field_serializer("measurement_start_time", "test_start_time") | ||
| def format_ts(self, v: datetime) -> str: | ||
| return v.strftime("%Y-%m-%dT%H:%M:%SZ") |
| return int(asn_str.strip("AS")) | ||
|
|
||
|
|
||
| def validate_report_id(report_id: str) -> str: |
There was a problem hiding this comment.
Can we move this validation function closer to where it's being used up on top?
| return report_id | ||
|
|
||
|
|
||
| def validate(item: str, accepted: str, error_msg: str): |
There was a problem hiding this comment.
I believe this function does not behave as intended. It's unclear from the docstring what it should do. The typing suggests it's a str, but it's use seems to treat it as a list[str].
I suggest changing either the docstring or the typing or both.
There was a problem hiding this comment.
From its usage above (this means the order or the function definitions should be inverted), you are checking to make sure it contains only the characters in the list, so you should probably call this something like:
def is_in_charset(s: str, charset: str, error_msg: str):
| elif request.report_id: | ||
| log.info(f"get_measurement_meta {request.report_id} {input}") | ||
| msmt_meta = _get_measurement_meta_clickhouse( | ||
| db, request.report_id, request.input |
There was a problem hiding this comment.
One of either measurement_uid or report_id should be provided, so you are missing the extra else branch which should lead to 400 status code error.
2 participants
Fixes several issues in the oonimeasurents component
Rewrites the
/api/v1/measuremen_metahandler to be more similar to the monolith versionAlso ports some
measurement_metarelated tests to the microservicecloses #1011