Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

[order-watcher] Public function getStats #1118

Merged
merged 10 commits into from
Oct 15, 2018

Conversation

amaurer
Copy link
Contributor

@amaurer amaurer commented Oct 6, 2018

Description

Added public function to get order watcher counts so I can effectively scale order watcher instances based on watch loads.

Testing instructions

Function added to test suite, no real changes as it reads in internal property that is assumed properly maintained.

Types of changes

Added function for getWatchCounts in order watcher and one test.

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Prefix PR title with bracketed package name(s) corresponding to the changed package(s). For example: [sol-cov] Fixed bug.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@fabioberger
Copy link
Contributor

Thanks a lot for this @amaurer! Do you think there might be additional stats we'd want to track in the future? Trying to decide if we want to make this a more generic getStats method that initially returns orderCount but could eventually return additional stats.

@amaurer
Copy link
Contributor Author

amaurer commented Oct 7, 2018

That makes sense so that, it is expandable and won't break interfaces in consuming applications. Changed to getStats which returns a Stats object with orderCount as a property. @fabioberger

@amaurer amaurer changed the title [order-watcher] Public function getWatchCounts [order-watcher] Public function getStats Oct 7, 2018
packages/order-watcher/CHANGELOG.json Outdated Show resolved Hide resolved
packages/order-watcher/CHANGELOG.json Show resolved Hide resolved
packages/order-watcher/src/order_watcher/order_watcher.ts Outdated Show resolved Hide resolved
@fabioberger
Copy link
Contributor

Thanks for changing that @amaurer! Looks good, just minor fixes then we can merge and publish.

@fabioberger
Copy link
Contributor

Also, could you run yarn prettier from the monorepo root, as well as yarn lint?

@fabioberger
Copy link
Contributor

@amaurer thanks for addressing my comments. Can you also export Stats in the index.ts file?

Once this is done and the conflicts are resolved, I'll approve & merge.

@fabioberger fabioberger merged commit 79db714 into 0xProject:development Oct 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants