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

Use multiple price feeds always #64

Merged
merged 6 commits into from
Oct 4, 2024
Merged

Conversation

harisang
Copy link
Contributor

@harisang harisang commented Oct 4, 2024

So far, we have used an ordering of price feeds, in the sense that we query the first API, and if it succeeds, we stop, and only continue with the second price feed if the first one doesn't respond. This PR changes this by forcing the use of all available price feeds/apis always.

Copy link
Contributor

@fhenneke fhenneke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber stamping an merging this to continue testing.

Changes look reasonable but I do not know how to test this locally. I expect this to create lots of alerts.

for provider in self.providers:
try:
price = provider.get_price(price_params)
if price is not None:
return price, provider.name
prices.append((price, provider.name))
except Exception as e:
logger.error(f"Error getting price from provider {provider.name}: {e}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being an error message sounds a bit strange. Most errors should be caught explicitly and would be more of a info type thing.

@@ -290,15 +289,21 @@ def handle_fees(
)

def handle_prices(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name sounds a bit strange. Maybe store_prices would work.

)
self.log_message.append(
f"Token: {token_address}, Price: {price} ETH, Source: {source}"
)
except Exception as err:
logger.error(f"Error: {err}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.error(f"Error: {err}")
logger.error(f"Error writing prices to database: {err}")

@fhenneke fhenneke merged commit 21f1e64 into main Oct 4, 2024
3 checks passed
@fhenneke fhenneke deleted the use_multiple_price_fees_always branch October 4, 2024 09:45
fhenneke added a commit that referenced this pull request Oct 4, 2024
…es_always"

This reverts commit 21f1e64, reversing
changes made to a8814b2.
@fhenneke fhenneke mentioned this pull request Oct 4, 2024
fhenneke added a commit that referenced this pull request Oct 4, 2024
@fhenneke fhenneke restored the use_multiple_price_fees_always branch October 4, 2024 12:19
@fhenneke
Copy link
Contributor

fhenneke commented Oct 4, 2024

Should have probably pushed a different button to revert this. Not sure how to reopen the PR now.

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