-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
perf: make silverback --help faster #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dtdang if you can just verify the CLI all works well still, we can merge
from .exceptions import CircuitBreaker, SilverbackException | ||
from .main import SilverbackBot | ||
from .state import StateSnapshot | ||
def __getattr__(name: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look like you are, but one thing to be aware of is autodoc won'tt work for lazy modules like this.
However, it isn't that hard to just add in the docs manually. Example: https://github.com/ApeWorX/ape/blob/main/docs/methoddocs/ape.md
Looks like some of the CLI commands that depend on ApePay are causing errors in the new PR since we refactored
|
|
Looks like I just needed to update the repo on my end, my bad. Testing the rest of the commands now but everything is looking good since pulling the latest repos. |
LGTM no issues with the commands since updating other repos. |
Well I am moving all the base model stuff to types since that is where yall went lookin for it: ApeWorX/ape#2353 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to double check this all still works before merging
OK but all I did was remove an import that no longer used. |
CLI is still working as intended |
The type annotation |
I am quite tempted to add more tests to silverback. |
Once I figure out how to do the whole backtesting thing, it'll be a lot easier. But testing the CLI would be good. |
What I did
Cuts the time down. It is twice as fast for me now to run
silverback --help
.How I did it
__getattr__
there.How to verify it
time silverback --help
Before:
After:
Results may vary.
Checklist