Conversation
yuvalrhino
left a comment
yuvalrhino
left a comment
There was a problem hiding this comment.
Very cool example!
Had a few comments - mainly around exception handling so that if something goes wrong users will get a more informative message
| - Access to Rhino FCP platform | ||
| - Access to a third party Secrets Manager platform (this example uses AWS Secrests Manager) | ||
|
|
||
| ## Dependencies |
There was a problem hiding this comment.
Do we need this section in the README? The requirements may change over time (e.g. due to vulnerabilities found in current versions of these packages) and then we'll likely update the requirements.txt, but may forget to update the README so it will get out of sync.
There was a problem hiding this comment.
Do we want the .enc version of this file in the repo? It's encrypted with specific keys, so probably not very helpful to have it here, right?
There was a problem hiding this comment.
Makes sense to remove it. The other encryption examples have them too, that's why I kept it.
| if 'encrypt_key' not in params: | ||
| raise ValueError("encrypt_key not found in secret params") | ||
|
|
||
| return RSA.import_key(params['encrypt_key']) |
There was a problem hiding this comment.
Nit: Perhaps put this in a try/except like you do in decrypt_code.py such that if the encrypt_key is in the wrong format this will show an informative error message?
There was a problem hiding this comment.
Do we need the .enc version of this file in the repo?
| } | ||
|
|
||
| # Create new secret in AWS Secrets Manager | ||
| self.client.create_secret( |
There was a problem hiding this comment.
This should be wrapped in a try/except in case there is a connection/permissions/format issue.
| ROLE_NAME = '<role_name>' | ||
|
|
||
| # Initialize secrets manager | ||
| secrets = SecretsManager( |
There was a problem hiding this comment.
There should be a try/except here in case initialization fails (e.g. if they forgot to update the ACCOUNT_ID and ROLE_NAME)
| ) | ||
|
|
||
| # Read input file | ||
| with open(input_file, 'rb') as f: |
There was a problem hiding this comment.
There should be a try/except here in case the input_file isn't found or can't be opened/read
| # Load private key from JSON | ||
| with secret_run_params_file_path.open("r") as secret_run_params_file: | ||
| secret_run_params = json.load(secret_run_params_file) | ||
| private_key = RSA.import_key(secret_run_params["decrypt_key"]) |
There was a problem hiding this comment.
There should be a try/except in case the decrypt_key isn't present in the secret_run_params or isn't in the correct format.
| private_key = RSA.import_key(secret_run_params["decrypt_key"]) | ||
|
|
||
| # Read the encrypted file | ||
| with open(model_parameters_path, 'rb') as f: |
Co-authored-by: Yuval Baror <78553214+yuvalrhino@users.noreply.github.com>
2 participants
No description provided.