Page MenuHomePhabricator

Pass conduit credentials down to children workflow

Authored by vm on Aug 28 2014, 7:16 PM.
Referenced Files
Unknown Object (File)
Wed, Mar 29, 9:16 AM
Unknown Object (File)
Sun, Mar 26, 11:59 PM
Unknown Object (File)
Sun, Mar 26, 5:24 PM
Unknown Object (File)
Sun, Mar 26, 2:31 PM
Unknown Object (File)
Wed, Mar 15, 3:34 PM
Unknown Object (File)
Mar 2 2023, 12:58 AM
Unknown Object (File)
Feb 28 2023, 9:53 AM
Unknown Object (File)
Feb 12 2023, 5:55 AM



Calling arc patch on a diff that's dependent on a different diff tries to patch the parent first.
To patch the parent a child workflow is created, and a conduit is passed down, but the credentials are not and it is not marked as authenticated.

Then when the child tries to get the commit message for the dependency, it checks isConduitAuthenticated();2c3268f03ed70d3221eb1642bIc99ebb39b12902e$800 and on failure pops up an interactive editor for the commit message.

Instead we just pass down the credentials to the childred and mark them as authenticated, so this is not a problem.

Test Plan

With two diffs where DA2 depends on DA1, run arc patch --force --nobranch DA2 ... this no longer pops an interactive editor for the commit message for the dependency.

Diff Detail

rARC Arcanist
Lint Skipped
Tests Skipped

Event Timeline

vm retitled this revision from to Pass conduit credentials down to children workflow.
vm updated this object.
vm edited the test plan for this revision. (Show Details)
vm added reviewers: epriestley, chad.
hach-que added inline comments.
800–802 ↗(On Diff #24987)

Remove debugging code?

800–802 ↗(On Diff #24987)

Oops. Thought I did.

vm edited edge metadata.

Remove debugging code

Ping, can we get this in soon? We'd like to support dependent diffs in our automated testing workflow (which relies on arc patch and hence needs diff).

chad removed a reviewer: chad.
epriestley edited edge metadata.

Possibly conduitURI and forcedConduitVersion should be getting passed down too, although there is probably no effect in practice.

T2247 discusses a flavor of this case in more detail.

We also plan to simplify the login/credentials dance a bit (T5955).

Anyway, this seems fine for the moment.

This revision is now accepted and ready to land.Sep 8 2014, 2:42 PM
epriestley updated this revision to Diff 25112.

Closed by commit rARC1b8ce98304f5 (authored by @vm, committed by @epriestley).