Update Scripts to integrate AWS S3 #807

jaredbradley243 · 2023-12-25T23:25:14Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
This PR modified the ingest and open_ai_func files to integrate AWS S3 into DocsGPT
Why was this change needed? (You can also link to an open issue here)
🚀 Feature: Use S3- bucket as the vector store #724
Other information:

vercel · 2023-12-25T23:25:19Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nextra-docsgpt	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 25, 2023 11:26pm

vercel · 2023-12-25T23:25:20Z

@jaredbradley243 is attempting to deploy a commit to the Arc53 Team on Vercel.

A member of the Team first needs to authorize it.

codecov · 2023-12-25T23:28:04Z

Codecov Report

Attention: Patch coverage is 0% with 93 lines in your changes are missing coverage. Please review.

Project coverage is 19.55%. Comparing base (7f79363) to head (30f2171).
Report is 180 commits behind head on main.

Files	Patch %	Lines
scripts/ingest.py	0.00%	59 Missing ⚠️
scripts/parser/open_ai_func.py	0.00%	34 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #807      +/-   ##
==========================================
- Coverage   19.56%   19.55%   -0.01%     
==========================================
  Files          62       72      +10     
  Lines        2914     3340     +426     
==========================================
+ Hits          570      653      +83     
- Misses       2344     2687     +343

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jaredbradley243 · 2023-12-25T23:28:10Z

Script Functionality

Local Mode (default): Processes documents from local directories specified by the user.
S3 Mode (--s3):
- Downloads documents from an S3 bucket to a temporary local storage (s3_temp_storage).
- Processes these documents.
- Uploads the processed documents back to the S3 bucket.

Enabling S3 Storage

To enable S3 storage, use the --s3 flag when running the script.

Environment Variables: Set these variables in your .env file:
- S3_BUCKET: Name of your S3 bucket.
- S3_DOCUMENTS_FOLDER: Folder within the S3 bucket where your documents are stored. If left blank, all files in S3 bucket will be downloaded (except .faiss and .pkl).
- S3_SAVE_FOLDER: Folder within the S3 bucket in which you would like to save the vector files. Leave blank to use the root of the bucket.
Running the Script:
- python ingest.py ingest --s3

Enabling Role Assumption

If accessing an S3 bucket requires assuming an IAM role (e.g., for cross-account access), the script supports this through the --s3-assume flag and proper AWS configuration.

Environment Variable:

Add AWS_ASSUME_ROLE_PROFILE to your .env file with the name of the AWS profile for role assumption. Ex: AWS_ASSUME_ROLE_PROFILE="dev"

AWS Configuration:

Credentials File (~/.aws/credentials):

[default]
aws_access_key_id = YOUR_DEFAULT_ACCESS_KEY
aws_secret_access_key = YOUR_DEFAULT_SECRET_KEY

[iamadmin]
aws_access_key_id = EXAMPLEKEY123456
aws_secret_access_key = EXAMPLESECRETKEY123456

Config File (~/.aws/config):

[default]
region = us-west-2
output = json

[profile dev]
region = us-west-2
role_arn = arn:aws:iam::123456789012:role/YourRoleName
source_profile = iamadmin

Running the Script with Role Assumption:
- python your_script.py --s3 --s3-assume

This configuration allows the script to assume YourRoleName using the credentials from the iamadmin profile.

Note

Ensure that the IAM role (YourRoleName) has necessary permissions for accessing the specified S3 bucket.
The script will create a temporary local storage (s3_temp_storage) for processing S3 documents, which will be cleaned up after processing.

larinam · 2024-01-21T19:28:58Z

To me, the following parameters look more logical to pass over command line parameters rather than as environment variables.
S3_BUCKET
S3_SAVE_FOLDER
AWS_ASSUME_ROLE_PROFILE

larinam · 2024-01-21T19:17:49Z

scripts/ingest.py

+from boto3.s3.transfer import TransferConfig
+
+# When override=True .env variables take precedence over environment. https://pypi.org/project/python-dotenv/
+dotenv.load_dotenv(override=True)


Why has this changed? Default logic looks reasonable.

Without override=true, the environment takes precedence over the .env variables. If a user should change their AWS credentials in the .env file, I wanted the change to take effect immediately. 😀

Sorry, I didn't get the justification and the use case.

tldr, if override is not set to true, the aws credentials in .env will not update each time the script is run.

Use case: User wants to change credentials in their env file between script runs.

I don't see your clear understanding of this change. I trust the default behaviour is more generally acceptable and in this case as well.
Please provide a clear case or remove this change.

scripts/ingest.py

scripts/parser/open_ai_func.py

jaredbradley243 · 2024-02-09T21:10:47Z

Thanks for the change requests, @larinam. I'll get to these soon!

jaredbradley243 · 2024-03-22T19:27:43Z

Working on these now.

jaredbradley243 · 2024-03-22T22:37:26Z

To me, the following parameters look more logical to pass over command line parameters rather than as environment variables. S3_BUCKET S3_SAVE_FOLDER AWS_ASSUME_ROLE_PROFILE

I added command line parameter support as well as env variables. 😁

This version includes support for command line arguments for s3_bucket, s3_documents_folder, s3_save_folder, s3_assume_role, aws_assume_role_profile S3 functionality has also been refactored into two functions, one for download located in ingest.py and one for upload located in open_ai_func.py

larinam · 2024-03-30T10:41:05Z

In general, according to the ticket, support for S3 should be added to the application as well. In this PR the changes are only in the scripts package. Please refer to the initial ticket for more information.

larinam · 2024-03-30T10:43:02Z

To avoid passing s3 config through several calls, please consider using https://docs.pydantic.dev/latest/concepts/pydantic_settings/
The configuration is static and doesn't change through the lifecycle of the script, so please consider using static object to manage these settings.

larinam

Please review the comments in the code and general comments.

larinam · 2024-03-30T10:15:40Z

scripts/ingest.py

+from boto3.s3.transfer import TransferConfig
+
+# When override=True .env variables take precedence over environment. https://pypi.org/project/python-dotenv/
+dotenv.load_dotenv(override=True)


I don't see your clear understanding of this change. I trust the default behaviour is more generally acceptable and in this case as well.
Please provide a clear case or remove this change.

larinam · 2024-03-30T10:30:30Z

scripts/parser/open_ai_func.py

+    aws_assume_role_profile = aws_assume_role_profile or os.environ.get("AWS_ASSUME_ROLE_PROFILE")
+
+    if not s3_bucket:
+        print("Error: S3_BUCKET environment variable is not set.")


Please get acquainted with the concept of Errors and Exceptions in Python.

Also the message is not completely correct in the context of the latest changes.

Update Scripts to integrate AWS S3

e08aa2d

github-actions bot added the scripts Scripts label Dec 25, 2023

vercel bot deployed to Preview – nextra-docsgpt December 25, 2023 23:26 View deployment

jaredbradley243 assigned dartpain Dec 25, 2023

larinam requested changes Jan 21, 2024

View reviewed changes

vercel bot deployed to Preview – nextra-docsgpt March 22, 2024 23:05 View deployment

jaredbradley243 requested review from larinam and dartpain March 22, 2024 23:10

jaredbradley243 assigned jaredbradley243 and unassigned dartpain and jaredbradley243 Mar 22, 2024

larinam requested changes Mar 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Scripts to integrate AWS S3 #807

Update Scripts to integrate AWS S3 #807

jaredbradley243 commented Dec 25, 2023

vercel bot commented Dec 25, 2023 •

edited

vercel bot commented Dec 25, 2023

codecov bot commented Dec 25, 2023 •

edited

jaredbradley243 commented Dec 25, 2023

larinam commented Jan 21, 2024

larinam Jan 21, 2024

jaredbradley243 Jan 21, 2024

larinam Jan 27, 2024

jaredbradley243 Mar 22, 2024

larinam Mar 30, 2024

jaredbradley243 commented Feb 9, 2024

jaredbradley243 commented Mar 22, 2024

jaredbradley243 commented Mar 22, 2024

larinam commented Mar 30, 2024

larinam commented Mar 30, 2024

larinam left a comment

larinam Mar 30, 2024

larinam Mar 30, 2024

larinam Mar 30, 2024

Update Scripts to integrate AWS S3 #807

Are you sure you want to change the base?

Update Scripts to integrate AWS S3 #807

Conversation

jaredbradley243 commented Dec 25, 2023

vercel bot commented Dec 25, 2023 • edited

vercel bot commented Dec 25, 2023

codecov bot commented Dec 25, 2023 • edited

Codecov Report

jaredbradley243 commented Dec 25, 2023

Script Functionality

Enabling S3 Storage

Enabling Role Assumption

Note

larinam commented Jan 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredbradley243 commented Feb 9, 2024

jaredbradley243 commented Mar 22, 2024

jaredbradley243 commented Mar 22, 2024

larinam commented Mar 30, 2024

larinam commented Mar 30, 2024

larinam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Dec 25, 2023 •

edited

codecov bot commented Dec 25, 2023 •

edited