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

Add PriceTracker to Price Oracle #403

Merged
merged 4 commits into from
Mar 13, 2023
Merged

Add PriceTracker to Price Oracle #403

merged 4 commits into from
Mar 13, 2023

Conversation

bbalser
Copy link
Contributor

@bbalser bbalser commented Mar 7, 2023

No description provided.

@jeffgrunewald
Copy link
Contributor

looks like this branch had an older commit from rg/price force-pushed to it; there are a bunch of small things that have been changed on that branch as a result of PR feedback that are still in play here so this should get rebased on the latest commit there unless it gets merged into main first

Copy link
Member

@madninja madninja left a comment

Choose a reason for hiding this comment

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

I don't quite get why price_tracker.rs is in the file_store at all.. We have none of the other services declare types here so why this one?

@jeffgrunewald
Copy link
Contributor

I don't quite get why price_tracker.rs is in the file_store at all.. We have none of the other services declare types here so why this one?

i wondered about that; seems like a reasonable if imprecise place to put it since it's technically just a client for streaming price_report files and there was little enough code required that it seems overkill to put it in its own crate

@bbalser bbalser force-pushed the bbalser/price_tracker branch from 94c316b to 19a95a7 Compare March 8, 2023 16:22
@bbalser
Copy link
Contributor Author

bbalser commented Mar 8, 2023

I don't quite get why price_tracker.rs is in the file_store at all.. We have none of the other services declare types here so why this one?

i wondered about that; seems like a reasonable if imprecise place to put it since it's technically just a client for streaming price_report files and there was little enough code required that it seems overkill to put it in its own crate

I originally created a new price_tracker crate, but it felt like overkill, but I can go back to that if we feel that is a better home.

@madninja
Copy link
Member

madninja commented Mar 8, 2023

I don't quite get why price_tracker.rs is in the file_store at all.. We have none of the other services declare types here so why this one?

i wondered about that; seems like a reasonable if imprecise place to put it since it's technically just a client for streaming price_report files and there was little enough code required that it seems overkill to put it in its own crate

I originally created a new price_tracker crate, but it felt like overkill, but I can go back to that if we feel that is a better home.

yes please? it's a running service/oracle right? not just a utility class

@bbalser
Copy link
Contributor Author

bbalser commented Mar 8, 2023

I don't quite get why price_tracker.rs is in the file_store at all.. We have none of the other services declare types here so why this one?

i wondered about that; seems like a reasonable if imprecise place to put it since it's technically just a client for streaming price_report files and there was little enough code required that it seems overkill to put it in its own crate

I originally created a new price_tracker crate, but it felt like overkill, but I can go back to that if we feel that is a better home.

yes please? it's a running service/oracle right? not just a utility class

It is a task that will be ran in other services/oracles just like FileSink

@bbalser bbalser changed the title Add PriceTracker to file_store Add PriceTracker to Price Oracle Mar 8, 2023
@bbalser bbalser marked this pull request as ready for review March 9, 2023 13:30
Base automatically changed from rg/price to main March 10, 2023 21:44
bbalser added 2 commits March 13, 2023 07:47
Update price tracker and have it fail it unable to get initial prices

appease clippy

Update PriceTracker to be in price oracle

Remove unused Error

Refactor to use more streaming
@bbalser bbalser force-pushed the bbalser/price_tracker branch from e56ce09 to f88bf9b Compare March 13, 2023 11:51
price/src/price_tracker.rs Outdated Show resolved Hide resolved
price/src/price_tracker.rs Outdated Show resolved Hide resolved
Co-authored-by: Matthew Plant <matty@nova-labs.com>
@bbalser bbalser merged commit bdcbd5f into main Mar 13, 2023
@bbalser bbalser deleted the bbalser/price_tracker branch March 13, 2023 13:06
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.

5 participants