Closed Bug 1400425 Opened 7 years ago Closed 7 years ago

Add a `mach try empty` command to push to try with no prompts

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla58

People

(Reporter: KWierso, Assigned: KWierso)

References

Details

Attachments

(1 file)

Bug 1400425 - Add a `mach try empty` command to push to try with no prompts
59 bytes, text/x-review-board-request
ahal
: review+
Details 
It might be useful to add a command similar to `mach try fuzzy`, but instead of getting prompted to select jobs to run, it just pushes the user's patches to try with no prompts, and scheduling nothing (except the decision task to build a list of runnable jobs, I guess?). After the push succeeds, the user can use Treeherder's Add New Jobs feature to select jobs to run.
Assignee: nobody → wkocher
Comment on attachment 8908915 [details]
Bug 1400425 - Add a `mach try empty` command to push to try with no prompts

This appears to work. I stripped down what fuzzy.py did.

This creates a try push like https://treeherder.mozilla.org/#/jobs?repo=try&revision=51fc8769a5dcdebd451d33f6103a73f2501ba70d which runs a decision task and a few other minor jobs, but no builds or tests.

Thoughts?
Attachment #8908915 - Flags: feedback?(gps)
Comment on attachment 8908915 [details]
Bug 1400425 - Add a `mach try empty` command to push to try with no prompts

https://reviewboard.mozilla.org/r/180532/#review185674

This seems like a reasonable implementation! Please send the final review to ahal.

::: tools/tryselect/selectors/empty.py:12
(Diff revision 1)
> +try:
> +    import blessings
> +    terminal = blessings.Terminal()
> +except ImportError:
> +    from mozlint.formatters.stylish import NullTerminal
> +    terminal = NullTerminal()

I don't think you need this.

::: tools/tryselect/vcs.py:104
(Diff revision 1)
>      def push_to_try(self, msg, labels=None, templates=None, push=True):
>          self.check_working_directory(push)
>  
>          config = None
> -        if labels:
> +
> +        if labels or labels == []:

Hmmm. I wonder if we could use ``labels or []`` below.
Attachment #8908915 - Flags: feedback?(gps) → feedback+
Comment on attachment 8908915 [details]
Bug 1400425 - Add a `mach try empty` command to push to try with no prompts

https://reviewboard.mozilla.org/r/180532/#review185686

Thanks! I've been meaning to add something like this, I think this is the right approach.

::: tools/tryselect/mach_commands.py:147
(Diff revision 2)
>          from tryselect.selectors.fuzzy import run_fuzzy_try
>          return run_fuzzy_try(**kwargs)
>  
>      @SubCommand('try',
> +                'empty',
> +                description='Push to try, running no builds or tests',

I'd call this `Push to try without scheduling any tasks`. There are a lot of tasks that are neither builds nor tests.

::: tools/tryselect/mach_commands.py:148
(Diff revision 2)
>          return run_fuzzy_try(**kwargs)
>  
>      @SubCommand('try',
> +                'empty',
> +                description='Push to try, running no builds or tests',
> +                parser=fuzzy_parser)

This argument needs to be removed, otherwise it will  accept the same cli that `fuzzy` does.

Doing this means there won't be any `kwargs`, which is I think what we want in this case.

::: tools/tryselect/selectors/empty.py:13
(Diff revision 2)
> +import subprocess
> +
> +from ..vcs import VCSHelper
> +
> +
> +def run(cmd, cwd=None):

Unused function?

::: tools/tryselect/selectors/empty.py:21
(Diff revision 2)
> +
> +
> +def run_empty_try(update=False, query=None, templates=None, full=False, parameters=None,
> +                  save=False, preset=None, list_presets=False, push=True, **kwargs):
> +    vcs = VCSHelper.create()
> +    vcs.check_working_directory(push)

This line should be removed, it'll happen as part of vcs.push_to_try(). It was only added to `fuzzy` so people don't get annoyed by having the tool bail out *after* selecting their tasks.

::: tools/tryselect/selectors/empty.py:23
(Diff revision 2)
> +    query = " with query: {}".format(query) if query else ""
> +    msg = "Pushed via 'mach try empty'{}".format(query)
> +    return vcs.push_to_try(msg, [], templates, push=push)

These lines can be turned into:

    return vcs.push_to_try("Pushed via `mach try empty`")

::: tools/tryselect/vcs.py:103
(Diff revision 2)
> -        if labels:
> +
> +        if labels or labels == []:

This is no longer necessary with the previous issue fixed.
Attachment #8908915 - Flags: review?(ahalberstadt)
Comment on attachment 8908915 [details]
Bug 1400425 - Add a `mach try empty` command to push to try with no prompts

https://reviewboard.mozilla.org/r/180532/#review185686

> These lines can be turned into:
> 
>     return vcs.push_to_try("Pushed via `mach try empty`")

I don't think I can make that change, given my comment below.

> This is no longer necessary with the previous issue fixed.

Without this change, I get | STOP! Either try_task_config.json must be added or the commit message must contain try syntax. | because we never write the empty task config file.
Comment on attachment 8908915 [details]
Bug 1400425 - Add a `mach try empty` command to push to try with no prompts

https://reviewboard.mozilla.org/r/180532/#review185686

> Without this change, I get | STOP! Either try_task_config.json must be added or the commit message must contain try syntax. | because we never write the empty task config file.

I want to make sure the try_task_config.json file is created, since this seems like it will be how future changes requiring `mach try` will be checked. If I'm wrong about those plans, let me know.
Comment on attachment 8908915 [details]
Bug 1400425 - Add a `mach try empty` command to push to try with no prompts

https://reviewboard.mozilla.org/r/180532/#review185976

Thanks for making these changes. Please fix the last two issues and this should be good to land.

::: tools/tryselect/selectors/empty.py:13
(Diff revisions 1 - 3)
>  def run_empty_try(update=False, query=None, templates=None, full=False, parameters=None,
>                    save=False, preset=None, list_presets=False, push=True, **kwargs):

These arguments can all be removed.

::: tools/tryselect/selectors/empty.py:17
(Diff revisions 1 - 3)
>  def run_empty_try(update=False, query=None, templates=None, full=False, parameters=None,
>                    save=False, preset=None, list_presets=False, push=True, **kwargs):
>      vcs = VCSHelper.create()
> -    vcs.check_working_directory(push)
>  
>      query = " with query: {}".format(query) if query else ""

Remove `query`.
Attachment #8908915 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8908915 [details]
Bug 1400425 - Add a `mach try empty` command to push to try with no prompts

https://reviewboard.mozilla.org/r/180532/#review185686

> I don't think I can make that change, given my comment below.

Ah, I agree with your comment below. Though you should still change it to what I suggested except with also passing in `[]`.

> I want to make sure the try_task_config.json file is created, since this seems like it will be how future changes requiring `mach try` will be checked. If I'm wrong about those plans, let me know.

Right, forgot about that.
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7fc37806848f
Add a `mach try empty` command to push to try with no prompts r=ahal
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/72ec5bce838a
Backed out changeset 7fc37806848f for flake8 issues a=backout
https://hg.mozilla.org/integration/autoland/rev/5084608dc0bb
Add a `mach try empty` command to push to try with no prompts r=ahal
https://hg.mozilla.org/mozilla-central/rev/5084608dc0bb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: