Add YH Finance (FINANCEAPI) API Key to Quotes infrastructure.

[jralls]

And add financeapi to known sources.
This to accommodate a new source in F::Q 1.63 that @bpschuck intends to release soon. There's some string revisions so we need to get this merged this weekend.

[gjanssens]

Looks good to me.

The use of boost::process::environment is a nice touch. Would it make sense to replace our use of getenv with calls to boost::process::environment as well for consistency ? One way to do that could be to make new_env a member variable of GncFQQuoteSource and populate it at construction time already. That would eliminate the need to store the two api keys themselves as member variables.

[jralls]

I used bp::environment because that's what bp::child wants for an environment argument. You're right, having retrieved the whole environment with bp::environment I should be using that to retrieve the api key from it instead of calling getenv.

Related to that, the current implementation prefers the preference value to the environment variable's value if both are set. Is that appropriate or should it be the other way around?

The idea to make new_env a member variable instead of keeping individual API keys as member variables is worth discussing. Having separate member variables for api keys isn't going to scale: There are already 8 (5 stock, 4 currency, but alphavantage is duplicated and yh finance will be). @bpschuck has in the past said that he'd prefer not to use environment variables for the keys--there's a way to set them in F::Q::new--and there are two of the 8 (CurrencyRates/Fixer.pm and CurrencyRates/OpenExchange.pm) that require an api key and lack an environment environment variable to set it. It would be more in line with F::Q's future direction to be passing a JSON dict of api keys to gnc-fq-wrapper.

Apropos that we also need to have a preference for the default currency source, and given Alpha Vantage's throttling and daily quote limits it should be something else.

There's not time for that this weekend. I'll merge this and do another PR.

[bpschuck]

On 9/20/24 11:15 AM, John Ralls wrote: @bpschuck <https://github.com/bpschuck> has in the past said that he'd prefer not to use environment variables for the keys--there's a way to set them in |F::Q::new|--and there are two of the 8 (CurrencyRates/Fixer.pm and CurrencyRates/OpenExchange.pm) that require an api key and lack an environment environment variable to set it. It would be more in line with F::Q's future direction to be passing a JSON dict of api keys to gnc-fq-wrapper.

Actually *I* was always in favor of environment variables. It was Vincent that had wanted to eliminate using environment variables. I'll add an issue for Fixer.pm and OpenExchange.pm to use an environment variable is one isn't passed in. F::Q v1.64 should have those fixes in place. I am open to discussing which would take precedence if both exist. I have no dog in that fight, so I'm good either way. Right now the modules that can utilize environment variables only do so if there was none passed in.

Apropos that we also need to have a preference for the default currency source, and given Alpha Vantage's throttling and daily quote limits it should be something else.

My vote would be to switch to FinanceAPI (hopefully v1.64 will have YH FinanceAPI Currency module) or CurrencyFreaks (available now. 1000 queries month with free key). Since CurrencyFreaks is currency conversion only, even now it's a better choice than AlphaVantage since all CurrencyFreaks queries would be currency conversion only. There is also a YahooJSON currency module, but as we've experienced, that can be subject to failure if Yahoo yet again makes changes. Happy Friday! Bruce S.

[jralls]

@gjanssens, of course using the bp::environment in get_api_key required creating it in the GncFQQuoteSource constructor, at which point I might as well make it a member and lose the api_key members, so I did. The result is cleaner still and @bpschuck's (thanks, Bruce!) clarification means that it's a good way forward.

[gjanssens]

@jralls regarding the question of precedence, I don't have a strong opinion. I can only imagine one use case where it would be useful if the environment value takes precedence over the preference value: perhaps someone may want to run gnucash-cli to retrieve quotes with a different-than default API key. In that case it would be convenient if it could just be set via an environment variable. I think that's rather uncommon though. And if that need really exists I'd probably rather implement it as an extra command line option to pass API key overrides.

For now my suggestion would be the current precedence rules are fine until someone presents a good case to do it differently.