buildstream-migration / bst-staging

GNU Lesser General Public License v2.1
0 stars 0 forks source link

Fail messages without logfile set cause traceback #619

Open Cynical-Optimist opened 4 years ago

Cynical-Optimist commented 4 years ago

See original issue on GitLab In GitLab by [Gitlab user @Qinusty] on Aug 30, 2018, 11:56

Summary

widget.py:L694 assumes that all FAIL messages coming from jobs (message.scheduler=True) have logfile set.

This is not asserted anywhere... This should be checked in the message API.

Steps to reproduce

Produce a fail message from within a job without setting logfile= in the constructor

What is the current bug behavior?

Traceback when buildstream attempts to get the last 20 lines in the log.

What is the expected correct behavior?

You should get an Assertion error from the message API.

Possible fixes

Other relevant information


Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @Qinusty] on Aug 30, 2018, 14:47

mentioned in commit f098654d654488eddb06be116a7a53d026664c17

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @Qinusty] on Aug 30, 2018, 16:24

mentioned in commit ee7ab32ea078ff1b394b67475392aa430d7a5555

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @Qinusty] on Aug 30, 2018, 17:07

mentioned in merge request !766

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @Qinusty] on Aug 30, 2018, 17:07

mentioned in commit 3a1659ffa741224c3d09922c8a3d0eee83bfb63a

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @Qinusty] on Aug 31, 2018, 07:37

mentioned in commit b887fe0779dc8b2e14ca9ccaa147138dd0109a7d

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @tristanvb] on Aug 31, 2018, 07:45

See my comment: https://gitlab.com/BuildStream/buildstream/merge_requests/766#note_98050404

If a FAILURE message is issued from a job, but is for some reason lacking a logfile, then this is most certainly a BUG; so a stack trace would be appropriate.

Why do we have FAILURE messages being issued without a logfile ? That seems to be the problem, not the fact that we crash when it's missing.

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @Qinusty] on Aug 31, 2018, 12:32

This should be addressed as part of the Message API rework. A BUG message should be raised if the BuildStream developer doesn't include a logfile in a FAILURE message.