[IT-4485] Upload CSV files from MIP to FloQast #2

ConsoleCatzirl · 2025-07-30T22:40:17Z

Create a Lambda to periodically fetch balance CSV data from lambda-mips-api and upload it to FloQast.

Depends on Sage-Bionetworks-IT/lambda-mips-api#50

Create a Lambda to periodically fetch balance CSV data from lambda-mips-api and upload it to FloQast.

coveralls · 2025-07-30T23:04:27Z

Pull Request Test Coverage Report for Build 17113633803

Details

92 of 103 (89.32%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-10.7%) to 89.32%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
floqast_sftp/app.py	92	103	89.32%

Totals
Change from base Build 16994745616:	-10.7%
Covered Lines:	92
Relevant Lines:	103

💛 - Coveralls

zaro0508 · 2025-08-11T17:16:30Z

README.md

-# lambda-template
-A GitHub template for quickly starting a new AWS lambda project.
+# lambda-finops-floqast-sftp
+AWS Lambda to periodically fetch balance CSV files from lambda-mips-api and upload


what are balance CSV files? elaboration would be nice.

The new lambda-mips-api endpoint for balances provides the data in CSV format, so I'm referring to the API response as CSV files here. I've reworded this to hopefully make it a little more clear.

template.yaml

zaro0508 · 2025-08-11T17:30:55Z

floqast_sftp/app.py

+    None
+
+    """
+    client.putfo(fl=file_obj, remotepath=name)


you should make sure to close the connection by wrapping this in try finally block. take a look https://stackoverflow.com/a/8293225/1094247

thanks, great catch! I wasn't closing the session anywhere. I do want to re-use the session for all of the uploads (each activity period has it's own CSV file) so I put the try/finally block around the main lambda_handler loop.

zaro0508 · 2025-08-11T17:34:31Z

template.yaml

+                "ssm_secret_prefix": "/floqast-sftp",
+                "period_count": "1",


shouldn't these be passed in as lambda parameters?

yeah, I was being lazy and hard-coded the values, but I've moved them to template parameters

zaro0508 · 2025-08-11T17:38:15Z

.coveragerc


 # Use 'source' instead of 'omit' in order to ignore 'tests/unit/__init__.py'
-source = hello_world
+source = floqast_sftp


hmm, can't seem to see line coverage in coveralls. it says source not available when click on the source files.

I can see it here for an out-of-date build: https://coveralls.io/builds/74889781
but it took a couple refreshes to get past an ajax error so maybe coveralls is having some sort of partial outage

zaro0508 · 2025-08-11T17:43:10Z

floqast_sftp/app.py

+    if "ssm_secret_prefix" not in event:
+        raise ValueError("'ssm_secret_prefix' not provided")
+    ssm_prefix = event["ssm_secret_prefix"]
+    auth = get_ssm_params(ssm_prefix)


What secrets are required for this lambda? It would help if you can provide some instructions in the docs on setting up these secrets.

Added to parameter documentation

zaro0508 · 2025-08-11T17:44:56Z

floqast_sftp/app.py

+
+    # number of months to get balances for
+    period_count = get_period_count(event)
+    logging.info(f"Getting balances for past {period_count} months")


this seems like an important setting for users however there's no docs for it?

Added to parameter documentation

zaro0508 · 2025-08-14T23:15:38Z

floqast_sftp/app.py

+
+ssm_client = None
+
+_csv_url = "https://mips-api.finops.sageit.org/balances?show_inactive_codes"


what do you think about passing in this var as a lambda parameter in case it ever changes?

my initial thinking was that it's just one line and easy to update if we need to. but it's probably worth it to make it a parameter so that if we do need to update it, we can do it from org-formation instead of needing to release a new lambda version.

zaro0508 · 2025-08-14T23:22:41Z

floqast_sftp/app.py

+            when = period.isoformat()
+            name, file_obj = get_balances_csv(when)
+            logging.info(f"Uploading {name} to SFTP server")
+            put_sftp_file(client, name, file_obj)


how about logging failures? also do you think we need to handle failures or just report them?

oh, great catch, we should definitely add more logging around upload errors, this is currently just relying on the lambda environment logging a generic exception and exiting.
but I'm not sure there's much handling we can do other than logging. and I would prefer for the exceptions to be fatal because if one upload fails, it's very unlikely that the next upload would succeed, and repeated failures might lock us out.

zaro0508

LGTM

dpulls · 2025-08-19T16:21:51Z

🎉 All dependencies have been resolved !

Copilot

Pull Request Overview

This PR transforms a lambda template project into a FloQast SFTP integration that periodically fetches trial balance CSV files from lambda-mips-api and uploads them to FloQast via SFTP. The Lambda is configured to run on a schedule and processes multiple months of financial data.

Replaces the template "hello world" functionality with SFTP file upload logic
Implements scheduled execution via EventBridge with configurable parameters
Adds comprehensive error handling and parameter validation for financial data processing

Reviewed Changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/test_handler.py	Comprehensive test suite covering SSM parameter handling, CSV processing, and Lambda handler scenarios
template.yaml	CloudFormation template updated for scheduled execution with SFTP-specific parameters and IAM configuration
hello_world/app.py	Removed original template handler code
floqast_sftp/app.py	New Lambda handler implementing SFTP client, CSV fetching, and file upload functionality
events/event.json	Updated test event structure for scheduled Lambda execution
README.md	Documentation updated with SFTP configuration, SSM parameters, and deployment instructions
Pipfile	Dependencies updated to include paramiko, requests, and testing libraries
.pre-commit-config.yaml	Updated hook versions and added black formatter
.coveragerc	Updated source directory reference

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

floqast_sftp/app.py

tests/unit/test_handler.py

floqast_sftp/app.py

* Close the SFTP session * Improve documentation * Parameterize inputs * Make the balances endopint URL configurable * Add upload failure logging * Expand some testing

* Add KMS key for encryting/decrypting SSM parameters * Fix parsing SSM parameter short names * Increase runtime timeout * Improve logging

ConsoleCatzirl requested review from a team as code owners July 30, 2025 22:40

ConsoleCatzirl force-pushed the initial branch 2 times, most recently from 65a5a47 to b43f063 Compare July 30, 2025 22:49

ConsoleCatzirl marked this pull request as draft July 30, 2025 22:49

[IT-4485] Upload CSV files from MIP to FloQast

5dd8da8

Create a Lambda to periodically fetch balance CSV data from lambda-mips-api and upload it to FloQast.

ConsoleCatzirl force-pushed the initial branch from b43f063 to 5dd8da8 Compare July 30, 2025 23:02

ConsoleCatzirl marked this pull request as ready for review July 31, 2025 16:21

ConsoleCatzirl requested a review from Copilot July 31, 2025 17:45

This comment was marked as outdated.

Sign in to view

zaro0508 requested changes Aug 11, 2025

View reviewed changes

ConsoleCatzirl force-pushed the initial branch 3 times, most recently from 461aeee to a04269b Compare August 12, 2025 19:33

ConsoleCatzirl requested a review from zaro0508 August 12, 2025 19:37

zaro0508 requested a review from a team August 13, 2025 16:43

zaro0508 requested changes Aug 14, 2025

View reviewed changes

ConsoleCatzirl requested a review from zaro0508 August 18, 2025 19:04

zaro0508 requested a review from a team August 18, 2025 22:06

zaro0508 approved these changes Aug 18, 2025

View reviewed changes

ConsoleCatzirl requested a review from Copilot August 19, 2025 16:23

Copilot AI reviewed Aug 19, 2025

View reviewed changes

floqast_sftp/app.py Outdated Show resolved Hide resolved

tests/unit/test_handler.py Outdated Show resolved Hide resolved

floqast_sftp/app.py Show resolved Hide resolved

Review feedback

75c3500

* Close the SFTP session * Improve documentation * Parameterize inputs * Make the balances endopint URL configurable * Add upload failure logging * Expand some testing

ConsoleCatzirl force-pushed the initial branch from 09ece9f to 75c3500 Compare August 19, 2025 16:59

Smoketest Debugging

1fff8d0

* Add KMS key for encryting/decrypting SSM parameters * Fix parsing SSM parameter short names * Increase runtime timeout * Improve logging

ConsoleCatzirl merged commit 0562f02 into Sage-Bionetworks-IT:main Aug 22, 2025
5 checks passed

ConsoleCatzirl deleted the initial branch August 22, 2025 17:35


		ssm_client = None

		_csv_url = "https://mips-api.finops.sageit.org/balances?show_inactive_codes"

[IT-4485] Upload CSV files from MIP to FloQast #2

[IT-4485] Upload CSV files from MIP to FloQast #2

Uh oh!

Conversation

ConsoleCatzirl commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 17113633803

Details

💛 - Coveralls

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConsoleCatzirl Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zaro0508 left a comment

Choose a reason for hiding this comment

Uh oh!

dpulls bot commented Aug 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ConsoleCatzirl commented Jul 30, 2025 •

edited

Loading

coveralls commented Jul 30, 2025 •

edited

Loading

ConsoleCatzirl Aug 12, 2025 •

edited

Loading