Skip to content

Conversation

escapewindow
Copy link
Contributor

Fixes #179.

This writes the chain of trust verification log to live_backing.log, then appends the task log. I could change this to write cot verification to live_backing.log, then move it to chain_of_trust.log on success, if we prefer not to see this output in the full log.

I'm thinking we should clean up/clarify the cot log output anyway, so maybe more visibility is better.

@escapewindow escapewindow requested a review from catlee June 1, 2018 22:20
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 6cbebc1 on escapewindow:cot-log into dd7b974 on mozilla-releng:master.

Copy link
Contributor

@catlee catlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to have the cot log be in a consistent place, rather than moving it depending on success/failure.

@escapewindow
Copy link
Contributor Author

This PR is on hold; @tomprince is working on bug 1400258.

@tomprince
Copy link
Contributor

The solution I had thought of is outlined here, but isn't as straightforward as I had hoped. I'm not actively working on this, but the first option I outlined there would be easy to implement, if that is the thought to be the best solution.

@escapewindow
Copy link
Contributor Author

escapewindow commented Jul 5, 2018

On June 1, I noticed that the chain_of_trust.log not showing up in treeherder's logs was causing the sheriffs some headaches. I wrote this patch since this solution seemed the fastest to implement; other than that I don't have strong opinions about which logfile the cot verification goes into.

I'm not sure if the sheriffs are still being negatively affected by the cot log not showing up in treeherder. (It's probably only really visible when cot verification fails.) We could land this and be done, or just leave it as is until one of us solves it another way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants