Hub model import script improvements #461
Merged
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
dolaru
force-pushed
the
hub_import_improvements
branch
from
05e99c3 to
0a69eb8
Compare
sethmlarson
reviewed
Apr 27, 2022
- Move bulky code out of `__main__` - Add support for authentication using username/password or api key - Graceful handling of missing PyTorch requirements - Graceful handling of already existing trained model - Use logging instead of print statements - Make logging a bit more verbose
dolaru
force-pushed
the
hub_import_improvements
branch
from
0a69eb8 to
987b759
Compare
sethmlarson
approved these changes
Apr 27, 2022
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.
Changes
Better logging
Switched from
printstatements tologgingfor a cleaner and more informative output - timestamps and log level are shown. The logging is now a bit more verbose, but it will help users to better understand what the script is doing.Add support for ES authentication using username/password or api key
Instead of being limited to passing credentials in the URL, there are now 2 additional methods:
--es-usernameand--es-password--es-api-keyCredentials can also be specified as environment variables with
ES_USERNAME/ES_PASSWORDorES_API_KEYGraceful handling of missing PyTorch requirements
In order to use the
eland_import_hub_modelscript, PyTorch extras are required to be installed. If the user does not have the required packages installed, a helpful message is logged with a hint to installeland[pytorch]withpip.Graceful handling of already existing trained model
If a trained model with the same ID as the one we're trying to import already exists, and
--clear-previouswas not specified, we now log a clearer message about why the script can't proceed along with a hint to use the--clear-previousflag.Prior to this change, we were letting the API exception seep through and the user was faced with a stack trace.
tqdmadded to main dependenciesIf the user doesn't have
eland[pytorch]extras installed, the first module to be reported as missing istqdm. Since this module is used in eland codebase directly, it makes sense to me to have it as part of the main set of requirements.Nit: Set tqdm unit to
partsin_pytorch_model.put_modelThe default unit is
it, butpartsbetter describes what the progress bar is tracking - uploading trained model definition parts.