Closed Bug 1191584 Opened 9 years ago Closed 9 years ago

The "comments" field is truncated to 1024 characters by the time it gets to mozharness

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(firefox42 fixed)

RESOLVED FIXED
Tracking Status
firefox42 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

Details

Attachments

(1 file)

The "comments" field for a build is potentially truncated by the time we see it in mozharness. This can be an issue for the "mach try" tool, which uses this to specify a (perhaps big) list of tests/manifests. We can probably work around this by getting the commit message from the pushlog or coming up with a new side-channel for test selection if this is a hard requirement.
Bug 1191584 - Get the full commit message from a try push in mozharness via pushlog to avoid truncations. r=jlund
Attachment #8644720 - Flags: review?(jlund)
https://reviewboard.mozilla.org/r/15313/#review13717 ::: testing/mozharness/mozharness/mozilla/testing/try_tools.py:24 (Diff revision 1) > + if len(msg) == 1024: > + # This commit message was potentially truncated, get the full message > + # from hg. > + props = self.buildbot_config['properties'] > + rev = props['revision'] > + repo = props['repo_path'] > + url = 'https://hg.mozilla.org/%s/json-pushes?changeset=%s&full=1' % (repo, rev) This is sort of unfortunate -- is there a better workaround available?
Comment on attachment 8644720 [details] MozReview Request: Bug 1191584 - Get the full commit message from a try push in mozharness via pushlog to avoid truncations. r=jlund https://reviewboard.mozilla.org/r/15313/#review13841 ah neat. too bad we cut off our commit message from the build side. IIRC this is because of how buildbot saves this to mysql and we hit a char limit and break the scheduler if it exceeds. have you tried this with > and < 1024 chars in try? ::: testing/mozharness/mozharness/mozilla/testing/try_tools.py:24 (Diff revision 1) > + if len(msg) == 1024: > + # This commit message was potentially truncated, get the full message > + # from hg. > + props = self.buildbot_config['properties'] > + rev = props['revision'] > + repo = props['repo_path'] > + url = 'https://hg.mozilla.org/%s/json-pushes?changeset=%s&full=1' % (repo, rev) not that I'm aware of. ::: testing/mozharness/mozharness/mozilla/testing/try_tools.py:37 (Diff revision 1) > + if not msg and 'try_syntax' in self.buildbot_config['properties']: should this be the default? as in, if try_syntax, use that? ::: testing/mozharness/mozharness/mozilla/testing/testbase.py:484 (Diff revision 1) > - comments = self.buildbot_config['sourcestamp']['changes'][-1]['comments'] > + self.parse_extra_try_arguments(known_try_arguments) ah, so this has side effect of mutating harness_extra_args and try_test_paths and the comments lookup happens within it now. maybe this method should be changed now to reflect that, like 'set_extra_try_args_from_commit_message'. or maybe that's verbose and obvious to someone familiar with TryToolsMixin
Attachment #8644720 - Flags: review?(jlund) → review+
(In reply to Jordan Lund (:jlund) from comment #4) > ah, so this has side effect of mutating harness_extra_args and > try_test_paths and the comments lookup happens within it now. maybe this > method should be changed now to reflect that, like > 'set_extra_try_args_from_commit_message'. or maybe that's verbose and > obvious to someone familiar with TryToolsMixin FWIW I have a patch that changes how this works so nothing mutates the arguments list in place but instead there's a method that returns extra harness arguments and a list of test locations to be run.
https://reviewboard.mozilla.org/r/15313/#review13863 ::: testing/mozharness/mozharness/mozilla/testing/testbase.py:484 (Diff revision 1) > - comments = self.buildbot_config['sourcestamp']['changes'][-1]['comments'] > + self.parse_extra_try_arguments(known_try_arguments) Right, I'll update the comment and call it something like "set_extra_harness_args". ::: testing/mozharness/mozharness/mozilla/testing/try_tools.py:37 (Diff revision 1) > + if not msg and 'try_syntax' in self.buildbot_config['properties']: Possibly, but I think the cases where I was using this the "comments" field was absent altogether.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: