-
Notifications
You must be signed in to change notification settings - Fork 100
Let Cocoon write into metrics center #1062
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
Conversation
This change shouldn't affect anything except for the "Linux benchmarks" LUCI post-submit bot. We'll closely monitor the LUCI bot to see if this works as intended. If so, we'll continue the migration in Cocoon (flutter/cocoon#1062) to cut the dependency on the old Cocoon datastore and Tong's desktop. Related issue: flutter/flutter#73872
4080a10 to
82ecfce
Compare
|
@keyonghan : I think this is now ready for review as flutter/engine#23767 has been successful on the engine side. |
keyonghan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with doc nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a doc briefly explain Metrics/flutter destination?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
liyuqian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review! Docs added :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the log, the error is DetailedApiRequestError, do you think we can use the exact error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use log.error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done.
This change shouldn't affect anything except for the "Linux benchmarks" LUCI post-submit bot. We'll closely monitor the LUCI bot to see if this works as intended. If so, we'll continue the migration in Cocoon (flutter/cocoon#1062) to cut the dependency on the old Cocoon datastore and Tong's desktop. Related issue: flutter/flutter#73872
This needs #1061 and the
metrics_centerchange inpubspec.yamlfrom #1060.Once this PR is successfully deployed, we should be able to cut the dependency on Tong's desktop.