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

Implemented S3 integration #23

Merged
merged 1 commit into from
Apr 24, 2024
Merged

Conversation

QuanMPhm
Copy link
Contributor

@QuanMPhm QuanMPhm commented Apr 22, 2024

Completing the first half of #21. I modified the invoice script to give the user the option to fetch invoices from S3 storage, as well as to upload them to S3.

The filenames for invoices to be stored locally and in S3 is determined by the user. The script expects several env vars to be sent to perform authentication to S3

The connection to Backblaze B2 is done in the Python script through boto3.

This PR will be followed-up with a PR to containerize the invoice processing.

Dockerfile Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
tools/clone_nonbillables_and_process_invoice.sh Outdated Show resolved Hide resolved
@QuanMPhm
Copy link
Contributor Author

@knikolla The build action currently does not work because the repo is missing a old_pi.csv file. This should be resolved once we know where that file should be fetched from.

Dockerfile Outdated Show resolved Hide resolved
@knikolla
Copy link
Contributor

@knikolla The build action currently does not work because the repo is missing a old_pi.csv file. This should be resolved once we know where that file should be fetched from.

See comment above. That file shouldn't be part of container image.

process_report/process_report.py Outdated Show resolved Hide resolved
@knikolla knikolla changed the title Containerized billing and implemented S3 integration Implemented S3 integration Apr 22, 2024
process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Show resolved Hide resolved
@QuanMPhm QuanMPhm marked this pull request as ready for review April 23, 2024 18:42
Copy link
Contributor

@knikolla knikolla left a comment

Choose a reason for hiding this comment

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

Almost there, just need to clean up the diff from changes that are no longer relevant.

process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
tools/clone_nonbillables_and_process_invoice.sh Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
Copy link
Contributor

@knikolla knikolla left a comment

Choose a reason for hiding this comment

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

Please update the description and answer the last few comments, and then once Naved gives his OK we can merge.

process_report/tests/unit_tests.py Show resolved Hide resolved
tools/clone_nonbillables_and_process.sh Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
Copy link
Contributor

@knikolla knikolla left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks.

Would like a final sign-off from @naved001 too.

Copy link
Collaborator

@naved001 naved001 left a comment

Choose a reason for hiding this comment

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

mostly looks good, just a couple of comments. Thanks!

process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
This commit gives the user the option fetch invoices from S3 storage. Several env vars are needed to authenticate to S3 storage.
More details on these vars can be found in the function `get_invoice_bucket()` from `process_report.py`
Note that the user can provide filenames containing "{}" to inject the invoice month, allowing for some convenient formatting
@QuanMPhm QuanMPhm merged commit d61d2ba into CCI-MOC:main Apr 24, 2024
2 checks passed
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.

3 participants