Arvados

Updates at 96f657210 - branch 13147-arvput-request-id
Test run: https://ci.curoverse.com/job/developer-run-tests/632/

Created a custom log formatter that appends the request-id on error messages. This formatter is used to replace the arvados formatter when running arv-put.

Wouldn't the X-Request-Id check always pass, because it always prints out the request id at the beginning? Maybe the test run should include "--silent"?

    def test_api_error_handling(self):
        coll_save_mock = mock.Mock(name='arv.collection.Collection().save_new()')
        coll_save_mock.side_effect = arvados.errors.ApiError(
            fake_httplib2_response(403), b'{}')
        with mock.patch('arvados.collection.Collection.save_new',
                        new=coll_save_mock):
            with self.assertRaises(SystemExit) as exc_test:
                self.call_main_with_args(['--silent', '/dev/null'])
            self.assertLess(0, exc_test.exception.args[0])
            self.assertLess(0, coll_save_mock.call_count)
            self.assertEqual("", self.main_stdout.getvalue())
            # Mock request id is added on log formatter at setUp
            self.assertRegex(
                self.main_stderr.getvalue(), r'\(X-Request-Id: req-testing123\)\n')

On further inspection, I guess that's not strictly true, because the regex is testing for "(X-Request-Id: req-testing123)" with the parenthesis as part of the match, which only shows up as part of an error message.

Another thought, if this adds the same request_id to every log message at ERROR or DEBUG level, that seems redundant. Maybe just add it once the first time?

Separately, I wonder if always logging X-Request-Id at INFO level at the beginning is desirable, even when the transfer was a success, but maybe that's a separate question (I don't want to get into it if it is going to be contraversial.)

Updates at 19ee2e029
Test run: https://ci.curoverse.com/job/developer-run-tests/635/ (waiting for workers atm)

Don't show X-Request-Id message on command startup.
Only show the request id once when logging error or debug messages.
Related tests added.

Lucas Di Pentima wrote:

Updates at 19ee2e029
Test run: https://ci.curoverse.com/job/developer-run-tests/635/ (waiting for workers atm)

Don't show X-Request-Id message on command startup.
Only show the request id once when logging error or debug messages.
Related tests added.

LGTM, please merge.