Reading default host as set by phabricator.uri or default config option

[robla]

As it stands, the default selected by keys()[0] will be arbitrary if there is more than one host

[BYK]

Hi there. I know this is a bit of a late response but are you still interested in this PR? This looks like a good idea but I'm not sure if those fields you use from .arcrc file are standard or not.

[robla]

It's been a while since I've played around with this library, but as I recall, I wrote this fix after several hours trying to figure out why the library "wasn't working". IIRC, using keys()[0] would effectively pick a random host from .arcrc because keys() is a dict, not a numeric array.

Regarding whether the fields from .arcrc file are standard or not, I'm not sure. I would have had an easier time if the library would have overtly failed rather than incorrectly reading phabricator.uri from .arcrc.

[BYK]

IIRC, using keys()[0] would effectively pick a random host from .arcrc because keys() is a dict, not a numeric array.

Oh yeah, that's true and I think this is a good case to throw an exception if we can't figure out anything certain on our own.

Those config keys kind of make sense to me. Pinging @avivey just in case though. I also want to keep the keys()[0] behavior if there's only one entry in the dict. Does that make sense?

Finally, are you still interested in getting this merged or want me to handle the rest?

[robla]

I don't have a strong opinion at this point, since my head isn't in this code atm.

[BYK]

Alright, then I'll take over your patch. Thanks :)

[avivey]

err... I'm actually not sure about reading .arcrc at all.

If the idea is for users to run programs based on this library, then it should also read .arcconfig and /etc/arcrc, etc, to better match the configuration arc is using (https://secure.phabricator.com/diffusion/ARC/browse/master/src/configuration/ArcanistConfigurationManager.php$21-26); But then I'd suggest you just write your script as an ArcanistWorkflow.

If the library is to be used almost always by non-humans, then using ~/.arcrc has some ops implications - the configuration needs to be kept in sync across hosts, using a different process than the code.

anyway, about the fields - those are the standard, yes. phabricator.uri has priority over default.

[BYK]

@avivey - Thanks!

We are reading all possible configuration files including the project-specific one: https://github.com/disqus/python-phabricator/blob/master/phabricator/__init__.py#L45-L65

[avivey]

Cool 👍