Skip to content
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

#132: Fixed outdated documentation #150

Merged

Conversation

MarleneKress79789
Copy link
Collaborator

@MarleneKress79789 MarleneKress79789 commented Nov 13, 2023

All Submissions:

  • Is the title of the Pull Request correct?
  • Is the title of the corresponding issue correct?
  • Have you updated the changelog?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Are you mentioning the issue which this PullRequest fixes ("Fixes...")
  • Before you merge don't forget to run tests in AWS CodeBuild, by adding [CodeBuild] to the commit message

Fixes #132

@@ -107,8 +109,7 @@ deployment script below with the desired version. (see GitHub Releases
--language-alias <LANGUAGE_ALIAS> \
--version <RELEASE_VERSION> \
--ssl-cert-path <ssl-cert-path> \
--use-ssl-cert-validation \
--no-use-ssl-cert-valiation
--use-ssl-cert-validation
```
The `--ssl-cert-path` is optional if your certificate is not in the OS truststore.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to say what certificate is this. There are two and they look similar. One is basically a list of trusted CA. It is needed for the server's certificate validation by the client (that's when you use the --use-ssl-cert-validation). Another one is the client's own certificate. It may or may not include the private key. In the latter case the key may be provided as a separate file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will add it to #133.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to elaborate on how to upload a model by downloading it from the Huggingface Hub to a local drive.

Firstly, a sample code like the one below would be useful. This code will create a model cache. This cache is actually what we need to upload to the BucketFS.

from transformers import AutoTokenizer, AutoModel

AutoTokenizer.from_pretrained(model_name, cache_dir=model_dir, token=user_token)
AutoModel.from_pretrained(model_name, cache_dir=model_dir, token=user_token)

For example, if model_name == 'me/my-awesome-model', and model_dir == 'my_model_dir' then the above code will create some files in the directory '.../my_model_dir/models--me--my-awesome-model'.

The '.../my_model_dir' is what we need to provide to the exasol_transformers_extension.upload_model in the local-model-path parameter. However, and this is important, it will grab EVERYTHING that it finds in this directory. If you downloaded several models there it will wrap them all in a single tar.gz file and start uploading. Therefore it is important to cache every model in its individual sub-directory. For example, if the root cache directory is 'my-models-cache' then I shall set the model_dir to something like '.../my-models-cache/my-awesome-model-cache'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you already mentioned that in #133. i don´t think it is relevant to this ticket, so i would suggest working on adding missing information to the documentation in #133 since that is the focus of that ticket.

@MarleneKress79789 MarleneKress79789 merged commit 57f9d33 into main Nov 14, 2023
3 checks passed
@MarleneKress79789 MarleneKress79789 deleted the documentation/132_fix_wrong_information_in_docu branch November 14, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix wrong information in Documentation
2 participants