Rush wrote on 2014-07-16 17:25:41 (UTC)

can someone point me to the current thing that does this for bugzilla?

demon wrote on 2014-07-16 18:41:29 (UTC)

https://gerrit.googlesource.com/plugins/its-bugzilla/ which relies on https://gerrit.googlesource.com/plugins/its-base/

aklapper wrote on 2014-08-11 19:58:25 (UTC)

After chat with Chase: Likely "patch_to_review" will become a tag in Phabricator, like a keyword in Bugzilla so we can search for tasks in that "project". (Also see T359.)

Still to sort out how to exactly/technically set and remove it.

mattflaschen wrote on 2014-08-12 02:10:11 (UTC)

Was the possibility of making it a status (like Bugzilla) discussed?

mattflaschen wrote on 2014-08-12 02:12:32 (UTC)

Sorry, I didn't mean to do those status changes.

I'm not sure who this was assigned to before, so assigning to @Aklapper.

aklapper wrote on 2014-08-12 08:14:01 (UTC)

In T277#15, @mattflaschen wrote:

Was the possibility of making it a status (like Bugzilla) discussed?

Yes (see T359), but a project/tag is way easier to archive once we have fixed T42.

aklapper wrote on 2014-09-01 19:31:00 (UTC)

http://git.wikimedia.org/blob/operations%2Fpuppet.git/d6a338ba8121dcf65b5f78e58876da63a9ce73e8/templates%2Fgerrit%2Fgerrit.config.erb#L131 has the configuration of the Bugzilla account which creates comments in Bugzilla based on related changes in Gerrit via http://git.wikimedia.org/blob/operations%2Fgerrit%2Fplugins.git/94b9a8afc7657839ab2a793c9b5a7ed222602b90/hooks-bugzilla.jar

Probably needs a similar section for [Phabricator]?

Also see T603.

(PS: Sorry for crappy long URLs but operations repo doesn't allow replacing those hashes via "HEAD")

demon wrote on 2014-09-01 19:46:27 (UTC)

Its more than just config. Someone will need to write the glue that translates the events into Phabricator API speak.

mattflaschen wrote on 2014-09-05 01:00:35 (UTC)

In T277#25, @Aklapper wrote:

(PS: Sorry for crappy long URLs but operations repo doesn't allow replacing those hashes via "HEAD")

You can use "production" instead of "HEAD".

aklapper wrote on 2014-09-08 13:50:21 (UTC)

In T277#26, @demon wrote:

Its more than just config. Someone will need to write the glue that translates the events into Phabricator API speak.

So we'll need a volunteer (assignee) here. Knowing Phab's API is obviously a need, and I wonder if @QChris (or Chad?) could provide resources and help sorting out the Gerrit part of it.

mmodell wrote on 2014-09-08 17:41:07 (UTC)

@Aklapper I could probably do this one I suppose.

Can someone elaborate on what exactly the plugin is supposed to do?

@mmodell: You might want to talk to qchris on IRC who also expressed some interest to investigate this task and should know the Gerrit part.

Potentially helpful for connecting to phabricator from gerrit: https://github.com/pirogoeth/jConduit

In order to run gerrit's bugzilla and gerrit's (not yet written) phabricator plugin in parallel,
I upgraded gerrit's hooks-bugzilla plugin to a version that adds the used Its to the events.
(See SAL at 2014-10-06 22:21 and https://gerrit.wikimedia.org/r/#/c/164879/ )

Using this version, we can effectively write gerrit actions that target only bugzilla, only
phabricator or both. Hence, we can run the plugins in parallel.
The preparation of the gerrit rule base is at https://gerrit.wikimedia.org/r/#/c/164881/ .

In order to get proper linking of tasks in gerrit, https://gerrit.wikimedia.org/r/#/c/164880/
teaches gerrit that “T” followed by at least one number should be linked to phabricator.
The upcoming its-phabricator will pick this up to map between commits and Phabricator
tasks.

Potentially helpful for connecting to phabricator from gerrit: https://github.com/pirogoeth/jConduit

Awesome!

However, I took a look at pirogoeth's jConduit, and I would not use it.

• jConduit does not come with a license (But since it's a github project that can probably be achieved easily).
• jConduit seems to contain code from others but without the original licensing (It's only a tiny part, but still ... having identified that even for a small part, the whole code comes with a legal “here be dragons” for me).
• I could not find jConduit packaged (Maven Central)
• jConduit comes with a home-brow dependency mananger
• jConduit is basically just two boilerplate files. It does not provide proper abstraction of Phabricator objects.

Due to the potential legal harm and due to the code not really
providing a proper abstractions of Phabricator objects, I would not
use jConduit at this point.

Another conduit binding for Java that I was pointed to, is
https://github.com/CodeBlock/Sconduit
It's scala, so interfacing would work out in principle. However, it's
README claims two known issues “Little error handling”, “No Java API”
:-(

Also, it's rather short code size wise, and does not provide a proper
abstraction. So I am not convinced that it's worth the Scala burden.

Then there is phabricator-java
https://github.com/creativepsyco/phabricator-java
which is just a README.md with License

Anyone know a nice Java library with Phabricator bindings?

I did some digging and came up with the same ideas you did :) so not much that is mature around I guess. I haven' t written any java in like 10 years but I'm wondering would it be a big deal to write a simple a phab class with an auth method? It should all be standard stuff.

the python-phabricator lib may give an idea

https://github.com/disqus/python-phabricator/blob/master/phabricator/__init__.py#L196

good luck man

In T169#2276, @flimport wrote:

After chat with Chase: Likely "patch_to_review" will become a tag in Phabricator, like a keyword in Bugzilla so we can search for tasks in that "project". (Also see T359.)

For the records, old T359 is now T212.

In T169#2276, @flimport wrote:

aklapper wrote on 2014-08-11 19:58:25 (UTC)

After chat with Chase: Likely "patch_to_review" will become a tag in Phabricator

This tag has just been created: https://phabricator.wikimedia.org/tag/patch-for-review/

Yesterday and today I finally found some hours to work on this, and I have a basic prototype running.
Gonna finish it up over the weekend.

I uploaded the plugin to https://gerrit.wikimedia.org/r/#/c/167525/ .

And the needed configuration updates are at https://gerrit.wikimedia.org/r/#/c/167524/ .
This last commit is still missing the username and certificate.

Do we already have a username and certificate for that we want to use for this bot, or should I create a new user?
If we need a new user, does phabricator require a wikitech user, or can it use a user that only lives in phabricator?

In T169#12056, @QChris wrote:

I uploaded the plugin to https://gerrit.wikimedia.org/r/#/c/167525/ .

And the needed configuration updates are at https://gerrit.wikimedia.org/r/#/c/167524/ .

Adding the Patch-For-Review tag manually. Pun very intended. :DDD

The code for the its-phabricator is (minus the backports to hooks-its) in the its-phabricator project of Google's gerrit gerrit:
https://gerrit-review.googlesource.com/#/c/60914/ .

In T169#12056, @QChris wrote:

Do we already have a username and certificate for that we want to use for this bot, or should I create a new user?

No bot account yet set up (querying https://phabricator.wikimedia.org/people/query/all/ shows 3 bots for me, none for Gerrit).

If we need a new user, does phabricator require a wikitech user, or can it use a user that only lives in phabricator?

I don't think that it requires a wikitech user, seeing e.g. the "wikiphabot" user here. Someone correct me if I'm wrong.

bots have no login / pass, only certificates and thus need no external auth provider :)

bots have no login / pass, only certificates and thus need no external auth provider :)

How does one get a sessionKey for a user without login?

(When I try to call the API's conduit.connect without passing a user, Phabricator responds with

"error_code": "ERR-INVALID-USER"
"error_info": "The username you are attempting to authenticate with is not valid."

. Also the the corresponding code in Phabricator's src/applications/conduit/method/ConduitConnectConduitAPIMethod.php
reads like there has to be a user.)

dude, I'm sorry my comment is confusing. I meant no login as in no local password. bots have usernames :)

@QChris I'll set up an account, you will need to use the conduit certificate to authenticate to phabricator.

Bot user: gerritbot

Bot user: gerritbot

Thanks. I updated https://gerrit.wikimedia.org/r/#/c/167524/3 accordingly.

@chasemp I don't have access to gerritbot's certificate, and also cannot access Ops' private repo since I am not Ops. Since
you can access both, could you take care of my comment about the certificate on https://gerrit.wikimedia.org/r/#/c/167524/3/templates/gerrit/secure.config.erb ?

should be available now as: $passwords::gerrit::gerrit_phab_cert

should be available now as: $passwords::gerrit::gerrit_phab_cert

Awesome! Thanks.

From my point of view, the puppet change to configure gerrit for phabricator is ready to go.

Chasemp merged the remaining change.
I deployed the its-plugin.
Tested adding a change and abandoning again in T775.
Seems to have worked.

I updated the Commit message guidelines.

Should we just let people toy with it, or should this be announced somewhere (wikitech-l)?

Thank you!

Please feel free to announce it in wikitech-l, maybe expressing that it is fresh, young, and welcoming users and feedback?

For instance, would it be possible to remove the "Patch-To-Review" project is a change has been abandoned? Although I can imagine a problem if two patches are being added to a task, and then only one of them is abandoned... Anyway, not urgent, this only shows my excitement about having this feature now working here. :)

@QChris: Will this work automatically in the transition period? I. e., after Bugzilla is switched read-only and a change is merged with "Bug: 4711", will the bot follow the redirect and post on the corresponding Phabricator task, or will unmerged changes with those footers need manual intervention? (Which would be okay, just requires someone to go through https://gerrit.wikimedia.org/r/#/q/status:open+message:%22Bug:+%255B0-9%255D%22,n,z.)

In T169#18062, @scfc wrote:

@QChris: Will this work automatically in the transition period? I. e., after Bugzilla is switched read-only and a change is merged with "Bug: 4711", will the bot follow the redirect and post on the corresponding Phabricator task, or will unmerged changes with those footers need manual intervention?

I expect manual intervention as it uses API that we cannot easily redirect.
I do not expect any notifications to work while Bugzilla and Phab are not available and/or read-only.

In T169#18592, @Aklapper wrote:

In T169#18062, @scfc wrote:

@QChris: Will this work automatically in the transition period? I. e., after Bugzilla is switched read-only and a change is merged with "Bug: 4711", will the bot follow the redirect and post on the corresponding Phabricator task, or will unmerged changes with those footers need manual intervention?

I expect manual intervention as it uses API that we cannot easily redirect.
I do not expect any notifications to work while Bugzilla and Phab are not available and/or read-only.

I agree with Aklapper.
Actually, as soon as we make bugzilla read-only, I'd stop the gerrit<->bugzilla bot. It couldn't do anything meaningful any longer.

When is this going to be enabled?

Change 169252 had a related patch set uploaded (by Tim Landscheidt):
fwconfigtool: Fix pyflakes warnings

https://gerrit.wikimedia.org/r/169252

Patch-For-Review

In T169#781460, @Esanders wrote:

When is this going to be enabled?

Seems to work.