Refactoring the rest of the commands and added the websocket for the assets for speed imporvments #93
Conversation
There was a problem hiding this comment.
Great stuff! Really happy you're reporting a major speed increase. Most of the comments I have in here are nit picky and some are prob more for future PRs. The only major thing I'd say is that we're on opposite sides of the spectrum with error handling. I know you want to minimize try catch blocks. @mark-at-pieces should prob make a judgement call on if we should have more error handling or keep it minimalistic.
src/pieces/assets/assets_command.py
Outdated
|
|
||
| if not name and not classification: # If no name or no classification is provided | ||
| # Ask the user for a new name | ||
| name = input("Enter the new name for the asset[leave blank to keep the same]: ").strip() |
There was a problem hiding this comment.
Since we're refactoring user inputs should probably live in a UI component
src/pieces/commands/list_command.py
Outdated
| for i, name in enumerate(asset_list, start=1): | ||
| print(f"{i}: {name.get('name')}") | ||
| if i >= max_assets: | ||
| break |
|
I am sure that there will not be any errors to catch in the api calls even if it will be handled using the argparser in the pieces_argparser.py file |
mark-at-pieces
requested review from
mack-at-pieces,
mark-at-pieces and
caleb-at-pieces
| asyncio.set_event_loop(loop) | ||
| loop.run_forever() | ||
|
|
||
| async def open_websocket(self): |
There was a problem hiding this comment.
since opening a websocket takes a certain amount of time, this method can technically get called many times on startup, leading to multiple websocket connections being opened. One way that could help prevent this is by making these connection management functions private to its class.
There was a problem hiding this comment.
Why the startup function will be called multiple times and even if we have a check where it make sure that it is not connected
There was a problem hiding this comment.
Why the startup function will be called multiple times and even if we have a check where it make sure that it is not connected
We do have a check to see it's connected however this boolean will not flip until the websocket has fully connected ( which takes some time ).
Fixing this will prevent an issue from happening with this in the future.
| from pieces.settings import Settings | ||
|
|
||
| # Used to create a valid file name when opening to "Opened Snippets" | ||
| def sanitize_filename(name): |
There was a problem hiding this comment.
this strategy is probably okay however a better one might be to use url encoding for this.
For example, instead of encoding ' ' as '_' we can encode it as '%20'
There was a problem hiding this comment.
This is the name of the file!
No description provided.