1198086 - Add InboundAutolandTrigger resource to work with autoland to inbound

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P1)

Description

[Dan Minor [:dminor]]

At the moment we have a TryAutolandTrigger endpoint which is used to trigger autoland to try requests. This needs to be made generic so it can be used to trigger requests to autoland to inbound as well as to try.

Comment 1

[Dan Minor [:dminor]]

Attachment: MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

mozreview: Change TryAutolandTrigger endpoint to work with autoland to inbound (bug 1198086) r?mdoglio,smacleod

This changes the TryAutolandTrigger so it can be used to trigger requests to
autoland to inbound as well as to try.

Comment 2

[Dan Minor [:dminor]]

Comment on attachment 8652407 [details]
MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

mozreview: Change TryAutolandTrigger endpoint to work with autoland to inbound (bug 1198086) r?mdoglio,smacleod

This changes the TryAutolandTrigger so it can be used to trigger requests to
autoland to inbound as well as to try.

Comment 3

[Mauro Doglio [:mdoglio]]

Comment on attachment 8652407 [details]
MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

https://reviewboard.mozilla.org/r/17157/#review15373

Are we going to change the csm_level check to be based on the  destination repository?

Comment 4

[Dan Minor [:dminor]]

After discussion, we've decided to add a separate endpoint for autoland to inbound as well as a base class to hold code common to both InboundAutolandTrigger and TryAutolandTrigger.

Comment 5

[Dan Minor [:dminor]]

Comment on attachment 8652407 [details]
MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

This adds an InboundAutolandTrigger resource for autolanding to inbound along
with a BaseAutolandTrigger resource to hold common code between
InboundAutolandTrigger and TryAutolandTrigger.

Comment 6

[Mauro Doglio [:mdoglio]]

Comment on attachment 8652407 [details]
MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

https://reviewboard.mozilla.org/r/17157/#review15461

Ship It!

Comment 7

[Steven MacLeod [:smacleod]]

Comment on attachment 8652407 [details]
MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

https://reviewboard.mozilla.org/r/17157/#review15471

::: pylib/mozreview/mozreview/autoland/resources.py:38
(Diff revision 3)
> -
> +class BaseAutolandTriggerResource(WebAPIResource):

This should probably have a decent docstring explaining what it implements and how it should be inherited from to add a new endpoint.

::: pylib/mozreview/mozreview/autoland/resources.py:95
(Diff revision 3)
> +        # In order to display the fact that a build was kicked off in the UI,
> +        # we construct a change description that our TryField can render.
> +        changedesc = ChangeDescription(public=True, text='', rich_text=False)
> +        changedesc.record_field_change(fieldname,

This sort of thing won't matter at all once we stop using change descriptions for the display. Maybe we should add a TODO, or call it out in mdoglio's bug where he plans to change how we display things.

::: pylib/mozreview/mozreview/extension.py:97
(Diff revision 3)
>          autoland_request_update_resource,
>          batch_review_resource,
>          ldap_association_resource,
>          review_request_summary_resource,
>          try_autoland_trigger_resource,
>          import_pullrequest_trigger_resource,
>          import_pullrequest_update_resource,
> +        inbound_autoland_trigger_resource,

These should have been in alphabetical order, can you fix that while you're here?

::: pylib/mozreview/mozreview/extension.py:112
(Diff revision 3)
>          register_resource_for_model(AutolandRequest,
> +                                    inbound_autoland_trigger_resource)
> +        register_resource_for_model(AutolandRequest,

This is problematic, you can't register two different resources for the same model.

Looking at how this works, it seems it should only ever matter if someone calls `get_resource_for_object` directly with an instance of our `AutolandRequest` model, *or* if you return an `AutolandRequest` model in a resource response (which it then uses to serialize the model so it can output it).

All that being said this makes me think we should just *not* register either of the resources for the model. That way, if someone tries to serialize using the methods above it will fail fast, rather than serializing with the wrong resource.

Comment 8

[Dan Minor [:dminor]]

Comment on attachment 8652407 [details]
MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

This adds an InboundAutolandTrigger resource for autolanding to inbound along
with a BaseAutolandTrigger resource to hold common code between
InboundAutolandTrigger and TryAutolandTrigger.

Comment 9

[Steven MacLeod [:smacleod]]

Comment on attachment 8652407 [details]
MozReview Request: mozreview: add InboundAutolandTrigger resource (bug 1198086) r?mdoglio,smacleod

https://reviewboard.mozilla.org/r/17157/#review15587

::: pylib/mozreview/mozreview/autoland/resources.py:39
(Diff revision 4)
> +    """ This provides a base class for resources that allow Autoland requests
> +        to be triggered.

No space before starting the docstring text.

Also, I think "Base resource for Autoland request triggers" would fit on the single line (which I highly prefer) and captures the same meaning.

::: pylib/mozreview/mozreview/autoland/resources.py:57
(Diff revision 4)
> -                           'land.',
> -        },
> +        #       descriptions to render autoland results. Once Bug 1176330 is
> +        #       this code can be removed.

"Once bug is [fixed?]"?

Comment 10

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/17157/#review15605

::: pylib/mozreview/mozreview/extension.py:26
(Diff revision 4)
> +                                          inbound_autoland_trigger_resource,

lets just call all the inbound autoland stuff "autoland" and treat "try_autoland" as the special snowflake.