1177945 - Allow automatically publishing a review request on push

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Botond Ballo [:botond]]

Now that reviewer deduction (bug 1142251) is in place, the next step I envision for improving the review submission workflow is to allow publishing the review request automatically when pushing. This would allow me as a review submitter to avoid having to interact with the MozReview web interface at all.

One way to do this would be to add a --publish flag to the push command:

  hg push review --publish

Comment 1

[Gregory Szorc [:gps]]

A lot of people asked for this at Whistler. I agree it is a significant paper cut and slows people down. I'll work on this.

Comment 2

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: clean up imports; r?smacleod

reviewboard: clean up imports; r?smacleod

The import of mercurial.i18n._ was added because it is used in this
file. Fortunately we shouldn't have hit those code paths in production.

Comment 3

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: properly send error responses; r?smacleod

reviewboard: properly send error responses; r?smacleod

wireproto.pusherr was being used to communicate errors to the client. The
way this works under the hood is that a literal string is sent to the
client, which is expected to infer that a string (as opposed to say an
integer in string form) is an error message. This completely confuses
the client-side parser, as it expects a newline based protocol.

Since pusherr is only really appropriate for errors during the push
operation, we leverage a custom type that has the same effect. It has
built-in knowledge of the wire protocol and automatically serializes to
use the "error" line command to represent the error.

Sadly, we have no test coverage for this. But an upcoming commit will
add some.

Comment 4

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: advertise features in mozreview capability; r?smacleod

reviewboard: advertise features in mozreview capability; r?smacleod

Adding separate capabilities for each reviewboard feature isn't the
preferred way to do things. Use a list of features in the "mozreview"
capability instead.

By committing only changes to the server, we prove that old clients are
compatible with the new server code.

Unfortunately, we can't easily repurpose the "reviewboard" capability
for the features list because client.py's auto redirect code effectively
does "if not caps['reviewboard'] and caps['listreviewrepos']" meaning
that adding "listreviewrepos" to the "reviewboard" capability wouldn't
satisfy the "not reviewboard" check and would cause auto discovery to
fail. Once sufficient time has passed and all clients in the wild have
been upgraded, we can consider removing this advertisement at the risk
of breaking old, un-upgraded clients.

Comment 5

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: look for capabilities in mozreview capability; r?smacleod

reviewboard: look for capabilities in mozreview capability; r?smacleod

Now that the server exposes a comma delimited list of features in the
"mozreview" capability, switch the client to use it.

Previously, presence of the "reviewboard" capability implied the ability
to push reviews. The equivalent is now the "pushreview" sub-capability.

This code is not backwards compatible with old servers. We should not
land this change until the server starts advertising the "mozreview"
capability.

Comment 6

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: verify capabilities when pulling review metadata; r?smacleod

reviewboard: verify capabilities when pulling review metadata; r?smacleod

We want a single code path for looking at capabilities and requirements.

Comment 7

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: check capabilities on client; r?smacleod

reviewboard: check capabilities on client; r?smacleod

The server now advertises a capability listing the required client
capabilities for interfacing with the server.

If clients see this capability, they will verify they meet the minimum
requirements to talk to the server and abort if not. Thus, all a server
needs to do to force all clients to upgrade is to advertise a new
required capability on the client.

Comment 8

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: require client to advertise capability knowledge; r?smacleod

reviewboard: require client to advertise capability knowledge; r?smacleod

The previous commit introduced "client capabilities" and a mechanism
where a server can advertise minimum required capabilities and clients
can abort if they are older than what the server requires. But there is
a bootstrap problem: old clients don't know to look for the minimum
required capabilities and thus ignore them completely. This could lead
to trouble.

With this commit, the client advertises to the server that it knows how
to apply capability requirements. If the server doesn't see this
advertisement, it assumes the request is coming from an old client and
will reject the request, telling the client to upgrade. Deployment of
this commit to the server thus constitutes a flag day and will require
all deployed clients to be upgraded. After this commit is deployed, it
is safe to assume that any newly-advertised requirements will be
respected by the client. This enables us to more quickly break backwards
compatibility with old clients since clients can easily be marked as
incompatible.

Comment 9

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: refactor update_review_request; r?smacleod

reviewboard: refactor update_review_request; r?smacleod

There is some duplicate code in this function around synchronizing the
state of review requests. Refactor update_review_request to take a
review request instance to prepare for removing the duplication.

Comment 10

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: remove duplicate code around review request updating; r?smacleod

reviewboard: remove duplicate code around review request updating; r?smacleod

Before this commit, the code for creating a new review request and
updating an existing review request was 95% similar. The only difference
was the function was storing the commit id and the in-line code
was populating it at review request creation time. In the end, it didn't
matter since the property was being assigned to the review request
draft.

The return value from update_review_request has been dropped since rr is
now passed in.

Comment 11

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: return review data as dicts, capture reviewers; r?smacleod

reviewboard: return review data as dicts, capture reviewers; r?smacleod

Before this commit, classes from the RBTools package were being returned
from reviewboardmods.pushhooks.post_reviews() and operated on inside the
Mercurial extension. This was arguably a layering violation. In
addition, since we don't import the rbtools modules from the Mercurial
extension, it meant we couldn't do things like catch the APIError from
the extension, which is needed for looking up the draft review request.
By returning dicts instead of RBTools classes, we work around both
deficiencies.

While we were here, reviewers are now returned from this function. This
will be used in a subsequent commit.

Comment 12

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: send reviewer info to client (bug 1177945); r?smacleod

reviewboard: send reviewer info to client (bug 1177945); r?smacleod

An upcoming commit will allow the client to publish review requests
during push. We want the client to be in control of this operation. To
enable the client to make this decision, we need to give it data.

This commit sends reviewer info to the client.

The "reviewdata" mechanism has grown the ability to handle lists in
addition to plain strings. Lists are simply comma delimited strings of
URL quoted values. It's worth noting that ',' is URL encoded to '%2C',
so looking for ',' in the value shouldn't have any false positives.

A new "listreviewdata" capability is now advertised as required to
ensure clients are capable of receiving lists in review data.

Comment 13

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/12299/#review10779

Work isn't complete. Submitting what I have so review can commence.

Comment 15

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: refactor common protocol formulation logic into function; r?smacleod

reviewboard: refactor common protocol formulation logic into function; r?smacleod

Protocol requests contain similar "header" data. Factor this into a
common function to cut down on code duplication.

Comment 16

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: refactor response parsing into common function; r?smacleod

reviewboard: refactor response parsing into common function; r?smacleod

This code is common and generic. Let's stop duplicating it.

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8627867 [details]
MozReview Request: reviewboard: require client to advertise capability knowledge; r?smacleod

reviewboard: require client to advertise capability knowledge; r?smacleod

The previous commit introduced "client capabilities" and a mechanism
where a server can advertise minimum required capabilities and clients
can abort if they are older than what the server requires. But there is
a bootstrap problem: old clients don't know to look for the minimum
required capabilities and thus ignore them completely. This could lead
to trouble.

With this commit, the client advertises to the server that it knows how
to apply capability requirements. If the server doesn't see this
advertisement, it assumes the request is coming from an old client and
will reject the request, telling the client to upgrade. Deployment of
this commit to the server thus constitutes a flag day and will require
all deployed clients to be upgraded. After this commit is deployed, it
is safe to assume that any newly-advertised requirements will be
respected by the client. This enables us to more quickly break backwards
compatibility with old clients since clients can easily be marked as
incompatible.

Comment 18

[Gregory Szorc [:gps]]

Comment on attachment 8627868 [details]
MozReview Request: reviewboard: refactor update_review_request; r?smacleod

reviewboard: refactor update_review_request; r?smacleod

There is some duplicate code in this function around synchronizing the
state of review requests. Refactor update_review_request to take a
review request instance to prepare for removing the duplication.

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 8627869 [details]
MozReview Request: reviewboard: remove duplicate code around review request updating; r?smacleod

reviewboard: remove duplicate code around review request updating; r?smacleod

Before this commit, the code for creating a new review request and
updating an existing review request was 95% similar. The only difference
was the function was storing the commit id and the in-line code
was populating it at review request creation time. In the end, it didn't
matter since the property was being assigned to the review request
draft.

The return value from update_review_request has been dropped since rr is
now passed in.

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 8627870 [details]
MozReview Request: reviewboard: return review data as dicts, capture reviewers; r?smacleod

reviewboard: return review data as dicts, capture reviewers; r?smacleod

Before this commit, classes from the RBTools package were being returned
from reviewboardmods.pushhooks.post_reviews() and operated on inside the
Mercurial extension. This was arguably a layering violation. In
addition, since we don't import the rbtools modules from the Mercurial
extension, it meant we couldn't do things like catch the APIError from
the extension, which is needed for looking up the draft review request.
By returning dicts instead of RBTools classes, we work around both
deficiencies.

While we were here, reviewers are now returned from this function. This
will be used in a subsequent commit.

Comment 21

[Gregory Szorc [:gps]]

Comment on attachment 8627871 [details]
MozReview Request: reviewboard: send reviewer info to client (bug 1177945); r?smacleod

reviewboard: send reviewer info to client (bug 1177945); r?smacleod

An upcoming commit will allow the client to publish review requests
during push. We want the client to be in control of this operation. To
enable the client to make this decision, we need to give it data.

This commit sends reviewer info to the client.

The "reviewdata" mechanism has grown the ability to handle lists in
addition to plain strings. Lists are simply comma delimited strings of
URL quoted values. It's worth noting that ',' is URL encoded to '%2C'
and all values should be URL encoded so looking for ',' in the value
shouldn't have any false positives.

A new "listreviewdata" capability is now advertised as required to
ensure clients are capable of receiving lists in "reviewdata" fields.

Comment 22

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: support for publishing review requests (bug 1177945); r?smacleod

reviewboard: support for publishing review requests (bug 1177945); r?smacleod

Having to visit a URL to publish review requests is annoying and wastes
developer time. Many developers have complained about this workflow.

This commit introduces support for publishing review requests when they
are submitted.

The server gains a wire protocol command for publishing review requests.
It advertises the presence of that command in the capabilities list.

Building on top of support for receiving reviewer information for
submitted review requests, the client now examines whether all submitted
review requests have reviewers attached. If so, it will prompt on
whether to publish.

I feel that people want the ability to auto publish during `hg push`.
This commit stops short of that and still requires a prompt. I have
concerns that there are enough bugs with commit mapping that have yet to
be worked out and that fully automatic publishing could result in too
much data loss. That is why auto publishing is not yet implemented.
However, depending on how well this step-in-the-right-direction feature
is received, we should consider adding it later if there is still
demand.

Comment 23

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/12411/#review10905

I'm still working out some test failures in this commit. But it should give you an idea of where I'm taking this feature. The series up to this point should be good for review. Although I may need to inject a commit in front of this one to fix an existing bug around draft detection.

Comment 24

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: consider draft review requests for public state; r?smacleod

reviewboard: consider draft review requests for public state; r?smacleod

As part of developing an unrelated feature, I discovered that the review
request "public" field sent to clients was wrong in the scenario where a
push had occurred to an existing review request. Essentially, the
"public" value was coming from the review request. If a review request
had never been published, it was always not public. If it had been
published, it was always listed as public.

This commit changes the behavior to take review request drafts into
consideration. If a draft exists, the "public" flag will be false.

As the output from the tests demonstrate, this bug has workflow
implications, as re-pushes to already published review requests may not
have been identified as "draft" as thus the user would not get prompted
to publish them.

One test change further demonstrates that the behavior on the server is
incorrect in the scenario where commits are re-pushed: if we perform a
"no-op" push, a new review request draft is created on the parent review
request. This is wrong and it has been reported in a new bug.

Comment 25

[Gregory Szorc [:gps]]

Comment on attachment 8627871 [details]
MozReview Request: reviewboard: send reviewer info to client (bug 1177945); r?smacleod

reviewboard: send reviewer info to client (bug 1177945); r?smacleod

An upcoming commit will allow the client to publish review requests
during push. We want the client to be in control of this operation. To
enable the client to make this decision, we need to give it data.

This commit sends reviewer info to the client.

The "reviewdata" mechanism has grown the ability to handle lists in
addition to plain strings. Lists are simply comma delimited strings of
URL quoted values. It's worth noting that ',' is URL encoded to '%2C'
and all values should be URL encoded so looking for ',' in the value
shouldn't have any false positives.

A new "listreviewdata" capability is now advertised as required to
ensure clients are capable of receiving lists in "reviewdata" fields.

Comment 26

[Gregory Szorc [:gps]]

Comment on attachment 8628543 [details]
MozReview Request: reviewboard: support for publishing review requests (bug 1177945); r?smacleod

reviewboard: support for publishing review requests (bug 1177945); r?smacleod

Having to visit a URL to publish review requests is annoying and wastes
developer time. Many developers have complained about this workflow.

This commit introduces support for publishing review requests when they
are submitted.

The server gains a wire protocol command for publishing review requests.
It advertises the presence of that command in the capabilities list.

Building on top of support for receiving reviewer information for
submitted review requests, the client now examines whether all submitted
review requests have reviewers attached. If so, it will prompt on
whether to publish.

I feel that people want the ability to auto publish during `hg push`.
This commit stops short of that and still requires a prompt. I have
concerns that there are enough bugs with commit mapping that have yet to
be worked out and that fully automatic publishing could result in too
much data loss. That is why auto publishing is not yet implemented.
However, depending on how well this step-in-the-right-direction feature
is received, we should consider adding it later if there is still
demand.

Comment 27

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/12299/#review10907

OK. I'm pretty happy with the state of this series now. Review away!

Comment 28

[(not currently active) Ted Mielczarek]

I was going to argue for auto-publishing, but having default-on prompt to publish is good enough for me!

Comment 29

[Steven MacLeod [:smacleod]]

Comment on attachment 8627861 [details]
MozReview Request: reviewboard: clean up imports; r?smacleod

https://reviewboard.mozilla.org/r/12303/#review11171

Ship It!

Comment 30

[Steven MacLeod [:smacleod]]

Comment on attachment 8627862 [details]
MozReview Request: reviewboard: properly send error responses; r?smacleod

https://reviewboard.mozilla.org/r/12305/#review11173

Ship It!

Comment 31

[Steven MacLeod [:smacleod]]

Comment on attachment 8627863 [details]
MozReview Request: reviewboard: advertise features in mozreview capability; r?smacleod

https://reviewboard.mozilla.org/r/12307/#review11175

Ship It!

Comment 32

[Steven MacLeod [:smacleod]]

Comment on attachment 8627864 [details]
MozReview Request: reviewboard: look for capabilities in mozreview capability; r?smacleod

https://reviewboard.mozilla.org/r/12309/#review11181

::: hgext/reviewboard/client.py:366
(Diff revision 1)
> -    assert remote.capable('reviewboard')
> +    assert getreviewcaps(remote)

This feels a little strange - wouldn't it be better to explicitly check the subset of capabilities which would allow this ('pushreview' or 'listreviewrepos')?  That way if we introduce a future capability, maybe an hg server for querying data only or something, it won't pass this check?

Comment 33

[Steven MacLeod [:smacleod]]

Comment on attachment 8627865 [details]
MozReview Request: reviewboard: verify capabilities when pulling review metadata; r?smacleod

https://reviewboard.mozilla.org/r/12311/#review11183

Ship It!

Comment 34

[Steven MacLeod [:smacleod]]

Comment on attachment 8627866 [details]
MozReview Request: reviewboard: check capabilities on client; r?smacleod

https://reviewboard.mozilla.org/r/12313/#review11185

Ship It!

Comment 35

[Steven MacLeod [:smacleod]]

Comment on attachment 8628541 [details]
MozReview Request: reviewboard: refactor common protocol formulation logic into function; r?smacleod

https://reviewboard.mozilla.org/r/12407/#review11189

::: hgext/reviewboard/client.py:84
(Diff revision 1)
> +def commonrequestlines(ui, bzauth):

Since we have cases where we just pass an empty bzauth dict, why don't we make it optional?

Comment 36

[Steven MacLeod [:smacleod]]

Comment on attachment 8628542 [details]
MozReview Request: reviewboard: refactor response parsing into common function; r?smacleod

https://reviewboard.mozilla.org/r/12409/#review11191

::: hgext/reviewboard/client.py:111
(Diff revision 1)
> +    assert version == 1

it might be worth sticking this protocol version in a constant somewhere (you *could* even format the `proto1` string using the constant too), that way we really only need to update the protocol version in one place.

Comment 37

[Steven MacLeod [:smacleod]]

Comment on attachment 8627867 [details]
MozReview Request: reviewboard: require client to advertise capability knowledge; r?smacleod

https://reviewboard.mozilla.org/r/12315/#review11193

Ship It!

Comment 38

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/12309/#review11181

> This feels a little strange - wouldn't it be better to explicitly check the subset of capabilities which would allow this ('pushreview' or 'listreviewrepos')?  That way if we introduce a future capability, maybe an hg server for querying data only or something, it won't pass this check?

We already do this check in a calling function. Although, it doesn't hurt to do it again. Will change in flight.

Comment 39

[Steven MacLeod [:smacleod]]

Comment on attachment 8627868 [details]
MozReview Request: reviewboard: refactor update_review_request; r?smacleod

https://reviewboard.mozilla.org/r/12317/#review11197

Ship It!

Comment 40

[Steven MacLeod [:smacleod]]

Comment on attachment 8627869 [details]
MozReview Request: reviewboard: remove duplicate code around review request updating; r?smacleod

https://reviewboard.mozilla.org/r/12319/#review11199

Ship It!

Comment 41

[Steven MacLeod [:smacleod]]

Comment on attachment 8627870 [details]
MozReview Request: reviewboard: return review data as dicts, capture reviewers; r?smacleod

https://reviewboard.mozilla.org/r/12321/#review11201

::: hgext/reviewboard/hgrb/proto.py:320
(Diff revision 2)
>          lines.extend([
>              'parentreview %s' % parentrid,
>              'reviewdata %s status %s' % (parentrid,
> -                urllib.quote(reviews[parentrid].status.encode('utf-8'))),
> -            'reviewdata %s public %s' % (parentrid, reviews[parentrid].public),
> +                urllib.quote(reviews[parentrid]['status'].encode('utf-8'))),
> +            'reviewdata %s public %s' % (parentrid,
> +                                         reviews[parentrid]['public']),
>          ])

the style of this thing is kind of a mess (before / after this change) - can we go with:
```
lines.extend([
    'parentreview %s' % parentrid,
    'reviewdata %s status %s' % (
        parentrid,
        urllib.quote(reviews[parentrid]['status'].encode('utf-8'))),
    'reviewdata %s public %s' % (
        parentrid,
        reviews[parentrid]['public']),
])
```

::: pylib/reviewboardmods/reviewboardmods/pushhooks.py:231
(Diff revision 2)
> +    def get_review_data(rr):

Please docstring this.

::: pylib/reviewboardmods/reviewboardmods/pushhooks.py:241
(Diff revision 2)
> +            if e.http_status != 404 or e.error_code != 100:

Comment above this line for `error_code` of 100 being Review Board API's code for "Does Not Exist" might be worth.

Comment 42

[Steven MacLeod [:smacleod]]

Comment on attachment 8628565 [details]
MozReview Request: reviewboard: consider draft review requests for public state; r?smacleod

https://reviewboard.mozilla.org/r/12445/#review11203

::: hgext/reviewboard/tests/test-push.t:111
(Diff revision 1)
> +TODO the behavior here is not correct: a new parent draft should not be
> +created if all the review requests didn't change

Changing this is going to require us to manually check all the differences ourselves. Review Board very much allows empty drafts (they'll fail to publish).

Comment 43

[Steven MacLeod [:smacleod]]

Comment on attachment 8627871 [details]
MozReview Request: reviewboard: send reviewer info to client (bug 1177945); r?smacleod

https://reviewboard.mozilla.org/r/12323/#review11205

Ship It!

Comment 44

[Steven MacLeod [:smacleod]]

Comment on attachment 8628543 [details]
MozReview Request: reviewboard: support for publishing review requests (bug 1177945); r?smacleod

https://reviewboard.mozilla.org/r/12411/#review11207

::: hgext/reviewboard/client.py:589
(Diff revision 2)
> +                    _('publish this review series (Yn)? $$ &Yes $$ &No'))

I think I'd prefer "publish this review request now"

::: hgext/reviewboard/client.py:603
(Diff revision 2)
> +    """Publish an iterable of review requests."""

"parent review requests" - attempting to publish a child from the API will fail.

But really, I'm tempted to say we should be using the review identifiers, rather than parent ids, for the new command. Do you have a good reason for not doing that?

Comment 45

[Steven MacLeod [:smacleod]]

Comment on attachment 8628543 [details]
MozReview Request: reviewboard: support for publishing review requests (bug 1177945); r?smacleod

https://reviewboard.mozilla.org/r/12411/#review11217

Ship It!

Comment 46

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/12411/#review11207

> "parent review requests" - attempting to publish a child from the API will fail.
> 
> But really, I'm tempted to say we should be using the review identifiers, rather than parent ids, for the new command. Do you have a good reason for not doing that?

As discussed on IRC, this isn't an issue now since we publish immediately after pushing. But in the future, we'll probably want to teach this wire protocol command how to accept a review identifier in addition to a review request id and have the server look up the review request id.

Comment 47

[Gregory Szorc [:gps]]

url:        https://hg.mozilla.org/hgcustom/version-control-tools/rev/c3d765303983f30dff6e4745664062e44dde52b8
changeset:  c3d765303983f30dff6e4745664062e44dde52b8
user:       Gregory Szorc <[e-mail]>
date:       Mon Jul 06 14:37:23 2015 -0700
description:
reviewboard: send reviewer info to client (bug 1177945); r=smacleod

An upcoming commit will allow the client to publish review requests
during push. We want the client to be in control of this operation. To
enable the client to make this decision, we need to give it data.

This commit sends reviewer info to the client.

The "reviewdata" mechanism has grown the ability to handle lists in
addition to plain strings. Lists are simply comma delimited strings of
URL quoted values. It's worth noting that ',' is URL encoded to '%2C'
and all values should be URL encoded so looking for ',' in the value
shouldn't have any false positives.

A new "listreviewdata" capability is now advertised as required to
ensure clients are capable of receiving lists in "reviewdata" fields.

url:        https://hg.mozilla.org/hgcustom/version-control-tools/rev/166a4edb819600ce134354559f4aa96445fe6577
changeset:  166a4edb819600ce134354559f4aa96445fe6577
user:       Gregory Szorc <[e-mail]>
date:       Mon Jul 06 15:15:43 2015 -0700
description:
reviewboard: support for publishing review requests (bug 1177945); r=smacleod

Having to visit a URL to publish review requests is annoying and wastes
developer time. Many developers have complained about this workflow.

This commit introduces support for publishing review requests when they
are submitted.

The server gains a wire protocol command for publishing review requests.
It advertises the presence of that command in the capabilities list.

Building on top of support for receiving reviewer information for
submitted review requests, the client now examines whether all submitted
review requests have reviewers attached. If so, it will prompt on
whether to publish.

I feel that people want the ability to auto publish during `hg push`.
This commit stops short of that and still requires a prompt. I have
concerns that there are enough bugs with commit mapping that have yet to
be worked out and that fully automatic publishing could result in too
much data loss. That is why auto publishing is not yet implemented.
However, depending on how well this step-in-the-right-direction feature
is received, we should consider adding it later if there is still
demand.

Comment 48

[Gregory Szorc [:gps]]

And this is deployed to production.

As part of the wire protocol refactoring, old clients will now be prompted to upgrade v-c-t. This means everyone should be upgraded to this new code on the next push to MozReview.

Comment 49

[Dan Minor [:dminor]]

I hit this just now (publishing by following the link to MozReview worked fine):

(venv)dminor@aragorn:~/src/version-control-tools$ hg push -r . review
pushing to ssh://reviewboard-hg.mozilla.org/version-control-tools
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 0 changes to 6 files (+1 heads)
remote: Trying to insert into pushlog.
remote: Inserted into the pushlog db successfully.
submitting 1 changesets for review

changeset:  3332:24159b18a663
summary:    autoland: remove testrun monitoring code (bug 1181139) r?mdoglio
review:     https://reviewboard.mozilla.org/r/12739 (draft)

review id:  bz://1181139/dminor
review url: https://reviewboard.mozilla.org/r/12737 (draft)

publish this review request now (Yn)? Y
error publishing review request 12737: You are not logged in (HTTP 401, API Error 103)
(review requests not published; visit review url to attempt publishing there)

Comment 50

[Gregory Szorc [:gps]]

I suspect what's happening is that the credentials are able to talk to Review Board but from Bugzilla's perspective they are stale. Another bug that goes away with Bugzilla tokens I reckon. Let's open a new bug or point people to the token bug if others run into this.

Comment 51

[(not currently active) Ted Mielczarek]

Filed bug 1184079.