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

us_hud income 20241127 changes #1126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kurus21
Copy link
Contributor

@kurus21 kurus21 commented Nov 27, 2024

No description provided.

Choose a reason for hiding this comment

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

Please add Copyright section.

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

Local requirement.txt file is missing. Can you add the file?

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

Still no requirement.txt file in change set!

if year < 2006:
return ''
suffix = str(year)[-2:]
if year == 2023:
return 'Section8-FY23.xlsx' # Directly reference 2023 file for download
elif year == 2024:

Choose a reason for hiding this comment

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

Why are the year and file name hardcoded? How will the script handle future years file?

Choose a reason for hiding this comment

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

The script does not include any error handling mechanisms. Please add try-catch blocks to handle potential exceptions and prevent the script from crashing unexpectedly.

Choose a reason for hiding this comment

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

The script currently lacks proper logging. This makes it difficult to troubleshoot issues and perform root cause analysis (RCA) in case of errors or unexpected behavior. Please add appropriate logging statements to track the script's execution and identify potential problems.

Copy link

@krishnaswamypradeep krishnaswamypradeep left a comment

Choose a reason for hiding this comment

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

Please work in the comments provided.

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.

4 participants