Skip to content

BDMS 121: log transfers#125

Merged
jirhiker merged 3 commits intopre-productionfrom
jab-transfer-logging
Sep 11, 2025
Merged

BDMS 121: log transfers#125
jirhiker merged 3 commits intopre-productionfrom
jab-transfer-logging

Conversation

@jacob-a-brown
Copy link
Contributor

Why

This PR addresses the following problem / context:

  • Transfer processes should be logged and persisted for understanding and audits

How

Implementation summary - the following was changed / added / removed:

  • Replaced all print statements with Python's logger

Notes

Any special considerations, workarounds, or follow-up work to note?

  • I'm currently using info for print statements and warning for when something goes wrong. Should we differentiate between warning and error?

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 3 files with indirect coverage changes

Copy link
Member

@jirhiker jirhiker left a comment

Choose a reason for hiding this comment

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

Is this PR up to date with the transfer branch?


if init or transfer_well_flag:
print("\n", "*" * 10, "TRANSFERRING WELLS", "*" * 10)
msg = "*" * 10 + "TRANSFERRING WELLS" + "*" * 10
Copy link
Member

Choose a reason for hiding this comment

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

message("TRANSFERRING WELLS")

def message(msg, pad='*', pad_len=10):
     pad *= pad_len
     logger.info(f"{pad} {msg} {pad}")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know of the transfer branch... I've just been working off of pre-production assuming that was the base for all of the project. Is that message function the only update? If so, I can also merge transfer into my branch, make updates/resolve conflicts, and push back to this PR

Copy link
Member

Choose a reason for hiding this comment

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

the other updates are to allow transfers to run via Cloud Run

@jirhiker jirhiker merged commit eebf76e into pre-production Sep 11, 2025
3 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