-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Alter time_elapsed field in BuildHistory Model #126
Conversation
Codecov Report
@@ Coverage Diff @@
## master #126 +/- ##
==========================================
+ Coverage 73.76% 73.84% +0.07%
==========================================
Files 45 46 +1
Lines 1334 1338 +4
==========================================
+ Hits 984 988 +4
Misses 350 350
Continue to review full report at Codecov.
|
If we are not doing any calculations over it and the format is not understood by the app/python directly, I'd say we should keep it simply as a string? |
It stores a |
It does. I wasn't aware of the format returned by the buildbot.
…On Sat, Sep 28, 2019 at 6:08 PM Arjun Salyan ***@***.***> wrote:
If we are not doing any calculations over it and the format is not
understood by the app/python directly, I'd say we should keep it simply as
a string?
It stores a timedelta object which does not support custom formatting. So
DurationField seems just the right choice for it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#126?email_source=notifications&email_token=ACFVMGDYIK7U32D7BLAIYM3QL5F3TA5CNFSM4I3LJQGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD72YXSY#issuecomment-536185803>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACFVMGA4SKY44WXEVK5D45TQL5F3TANCNFSM4I3LJQGA>
.
|
Is the migration really so simple and the conversion works out of the box? Will the app redo fetching data from the failed parsing cases? Looks ok to me in any case, duration makes more sense than useless string. |
Yes. We were already storing a
Yes, the app always starts fetching from I have also tested this on EC2. |
If this looks good, we should merge it asap. We are missing on updates due to this. |
Closes: #124