Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/holdings ignored #275

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tutilus
Copy link
Contributor

@tutilus tutilus commented Aug 1, 2024

Hi there,

As a fix to #245 issue and by extension to every sources which is not Yahoo! source, I propose this MR.

Looks like the holdings appear only in Yahoo! source because the key to look-for "holdings" in yaml-config-file variables is based on Symbol.
In Yahoo! source, the symbol is the id and Yahoo! API returns the same value as symbol. This is different with the other source where the symbol id to search on the API could be "cardona" and the symbol "ADA" (which is different to CARDONA.CG).

As proposed solution, I had an "Id" into AssetQuote as an equivalent to the "Symbol" value in Yaml config file and I provisioned this value from the "Symbol" declaration into the config file (I used a new c.Symbol type with "id" and "name").

I hope it could help to fix this issue.

@achannarasappa
Copy link
Owner

Thanks for making the effort to look into this issue. I'm not sure if it's possible to reproduce that issue with CoinGecko any longer since CoinGecko has now started gating their API with a key. I was thinking about removing CoinGecko support altogether since it does not work at the moment.

Also a there's a similar concept to the ID / symbol mapping being proposed here within the getSymbolBySource function unless I am misunderstanding the intent here. This function uses the suffix (e.g. .cg) to infer the source or use these shortcut symbols statically defined here to do the same

@tutilus
Copy link
Contributor Author

tutilus commented Sep 3, 2024

Hi Ani and thank you for your kind answer.

Maybe it would be a good idea to improve the way you manage source (as I think it was expected in #64) and allow using API key.

Anyway, I'm not sure it's specific to CoinGecko. In my understanding, symbols are based on different sources: config file and id provided by API response.
The only symbols kept in memory is the one from the API response in c.AssetQuote. But when it's getting the holding assets, it's based on the config file and it presume symbols are the same as in getHoldingFromAssetQuote. In Yahoo! it matches but in other case it doesn't. So my intent here is to keep the symbol declared in the config file as an id in the c.AssetQuote object to be sure to match in all cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants