sarthaksin1857 commented on code in PR #131:
URL: https://github.com/apache/otava/pull/131#discussion_r3105345385
##########
otava/graphite.py:
##########
@@ -80,49 +79,28 @@ class GraphiteError(IOError):
@dataclass
class GraphiteEvent:
- test_owner: str
- test_name: str
- run_id: str
- status: str
- start_time: datetime
pub_time: datetime
- end_time: datetime
- version: Optional[str]
- branch: Optional[str]
- commit: Optional[str]
-
- def __init__(
- self,
- pub_time: int,
- test_owner: str,
- test_name: str,
- run_id: str,
- status: str,
- start_time: int,
- end_time: int,
- version: Optional[str],
- branch: Optional[str],
- commit: Optional[str],
- ):
- self.test_owner = test_owner
- self.test_name = test_name
- self.run_id = run_id
- self.status = status
- self.start_time = parse_datetime(str(start_time))
- self.pub_time = parse_datetime(str(pub_time))
- self.end_time = parse_datetime(str(end_time))
- if len(version) == 0 or version == "null":
- self.version = None
- else:
- self.version = version
- if len(branch) == 0 or branch == "null":
- self.branch = None
- else:
- self.branch = branch
- if len(commit) == 0 or commit == "null":
- self.commit = None
- else:
- self.commit = commit
+ test_owner: Optional[str] = None
+ test_name: Optional[str] = None
+ run_id: Optional[str] = None
+ status: Optional[str] = None
+ start_time: Optional[datetime] = None
+ end_time: Optional[datetime] = None
+ version: Optional[str] = None
+ branch: Optional[str] = None
+ commit: Optional[str] = None
+
+
+ def __post_init__(self):
+ if self.pub_time is None:
+ raise ValueError("pub_time is required and cannot be None")
+ # Ensure pub_time is always a datetime
+
+ # Only parse if it isn't already a datetime object
+ if isinstance(self.pub_time, str):
+ self.pub_time = parse_datetime(self.pub_time)
+ elif isinstance(self.pub_time, (int, float)):
+ self.pub_time = datetime.fromtimestamp(self.pub_time)
Review Comment:
This was more just me looking at this code an how to future proof it.
The function says that pub_time is already a date time and if its already a
datetime, why try to parse it again?
Similarly if someone passes in a unix int like 1776522767 the string
function wont work either
So before we passed in an integer string as "datetime" and then parsed it
back into a string, and then into the proper object.
I will just move the logic into parse_datetime and make this code cleaner
--
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]
