Remove app creation at module level + refactoring #63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vblagoje
reviewed
Feb 6, 2025
| def deploy_files(server_conf, name, folder): | ||
| """Deploy all pipeline files from a folder to the Hayhooks server.""" | ||
| @click.argument('pipeline_dir', type=click.Path(exists=True, file_okay=False, dir_okay=True, path_type=Path)) | ||
| def deploy_files(server_conf, name, pipeline_dir): |
There was a problem hiding this comment.
Yes we can, but to tell you the truth I was planning to add fastapi[standard] as a dep and then rewrite the CLI commands using Typer, which is cleaner and has a stronger types support. I've already written it actually, but wanted to tackle those other refactoring first 😉
vblagoje
approved these changes
Feb 6, 2025
anakin87
approved these changes
Feb 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This may seems a bit convoluted, but actually contains minor refactoring and improvements:
appinstanceI've removed the
appcreation at module level inapp.py. We were instantiatingappeach time that module is imported and this is not optimal. Now we're using only thecreate_apphelper.This will allow users to programmatically create their
appinstance, adding required middlewares when needed (e.g. for CORS, authentication or authorization). It's also used inhayhooks runCLI command. This will be better documented on another PR.Test improvements
TestClientinstances around test files and used a single one as a fixture inconftest.py.settingsfor tests, creatingpipelinesdir intestsdir.pipelinesdir after each test module run.Folder -> Dir
There were many references to
folderanddirconcepts. I've chosendirand removedfolderreferences (but not yet inREADME.mdanddeployment_guidelines.md, another PR will take care of this).