Add columns and file for daily billable usage#133
Conversation
naved001
left a comment
naved001
left a comment
There was a problem hiding this comment.
looks good, just some optional suggestions
| secondary_location = ( | ||
| # Upload daily copy | ||
| # End time is exclusive, subtract one second to find the inclusive end date | ||
| invoice_date = end - timedelta(seconds=1) |
There was a problem hiding this comment.
Should end be converted to UTC first, and then we can subtract a day from it? Or can we have default_start_argument and default_end_argument be in UTC?
There was a problem hiding this comment.
@QuanMPhm All times processed in this script (event times for VMs coming from the database) are timezone-naive objects that are assumed to be in UTC. Comparing time-naive datetime objects with datetime objects that have a timezone associated to them leads to an error, hence the default_start_argument and default_end_argument don't have a timezone associated to them.
Though since they are fetched without providing tzinfo, they'll be in the local time of machine running the script. I could fetch them from datetime.now(timezone.utc) instead, which seems reasonable. But I'd still need to strip away the timezone to make them timezone-naive and reset them to 00:00:00.
Can you please explain your comment about subtracting 1 day instead of 1 second?
There was a problem hiding this comment.
Either way, as Naved is out today and tomorrow and these changes would re-require his approval if you're okay with the current state of this PR I can make them in a future PR.
There was a problem hiding this comment.
@knikolla I'm fine with the PR as it is. I'll approve it now.
Can you please explain your comment about subtracting 1 day instead of 1 second?
I had a misunderstanding about timestamps. I thought it you added timezone info to a naive timestamp, it would change the hour value for the timestamp. I didn't realize the timestamp would still numerically be the same, just no longer naive. You can disregard my comment.
3 participants
Report Start Time, Report End Time, Generated AtCloses #131
Related nerc-project/operations#1307