Restricting a bug's visibility does not delete any associated MozReview review requests

RESOLVED FIXED

Status

()

P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mcote, Assigned: dylan)

Tracking

Production

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
I pushed a test review request (https://reviewboard.mozilla.org/r/361) for an invalid bug (bug 1072809), published the review request, and then marked the bug as visible to MoCo employees only.  This should have deleted the review request as per bug 993223, but nothing happened; the review request still exists and is visible.

Several things have changed in MozReview since bug 993223 was fixed, so maybe something broke it.  I'm going to file a separate bug for a version-control-tools test.
(Assignee)

Comment 1

4 years ago
We need to configure the push connector for ReviewBoard. Needs a URL, username and password.
Assignee: nobody → dylan
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
Let me know if the push connector is successfully deleting reviews now. Thanks!
Flags: needinfo?(mcote)
(Reporter)

Comment 3

4 years ago
I made the bug public again and then put it back to MoCo only, but the review request continues to exist.
Flags: needinfo?(mcote)
(Assignee)

Comment 4

4 years ago
Hmm. I think the push connector needs the proxy setting for production. Weirdly, the push log is recording some transient errors:

https://bugzilla.mozilla.org/page.cgi?id=push_log.html
(Assignee)

Comment 5

4 years ago
After working through the misconfiguration, it was discovered that he version of LWP::UserAgent on prod does not support the ->delete() method. We can do one of two things:

1) monkey patch in a delete() method, which is defined in terms of HTTP::Request::Common::DELETE.
2) upgrade LWP::UserAgent.
(Assignee)

Comment 6

4 years ago
Patch to monkey-patch $ua->delete() in. I'm not incredibly fond of this idea, but I'm also not sure how reasonable it is to upgrade LWP::UserAgent on prod. (I'm also a little concerned that -dev/-staging have newer versions, as I remember this code running there.)
Attachment #8520349 - Flags: review?(glob)
Comment on attachment 8520349 [details] [diff] [review]
bug-1096318-v1.patch

Review of attachment 8520349 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Dylan William Hardison [:dylan] from comment #6)
> Created attachment 8520349 [details] [diff] [review]
> bug-1096318-v1.patch
> 
> Patch to monkey-patch $ua->delete() in. I'm not incredibly fond of this
> idea, but I'm also not sure how reasonable it is to upgrade LWP::UserAgent
> on prod.

i agree, but we're looking at a LWP 5 --> 6 upgrade here, which i've had problems with in the past.  pretty sure bugzilla is ok with lwp6, but given the monkeypatch is trivial it's the safer thing to do here.

> (I'm also a little concerned that -dev/-staging have newer
> versions, as I remember this code running there.)

[bjones@web3.stage.bugs.scl3 ~]$ perl -MLWP::UserAgent -e "LWP::UserAgent->delete"
Can't locate object method "delete" via package "LWP::UserAgent" at -e line 1.

[bjones@web5.stage.bugs.scl3 ~]$ perl -MLWP::UserAgent -e "LWP::UserAgent->delete"
Can't locate object method "delete" via package "LWP::UserAgent" at -e line 1.

::: extensions/Push/lib/Connector/ReviewBoard/Client.pm
@@ +23,5 @@
> +        *LWP::UserAgent::delete = sub {
> +            require HTTP::Request::Common;
> +            my($self, @parameters) = @_;
> +            my @suff = $self->_process_colonic_headers(\@parameters,1);
> +            return $self->request( HTTP::Request::Common::DELETE( @parameters ), @suff );

nit: this doesn't follow bugzilla's code style; fix on commit
Attachment #8520349 - Flags: review?(glob) → review+
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   2d223dc..5b9d96e  master -> master
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
now getting:
> garbage after JSON object, at character offset 4 (before "Can't connect to dc-...")

looks like the /etc/hosts entry for dc-proxy doesn't exist everywhere.
Depends on: 1096802
that's fixed, now getting:
> The username or password was not correct
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 11

4 years ago
It is really fixed this time.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.