Added pickles, auto-deployment #4
Conversation
WalkthroughThis pull request introduces several changes across multiple files. A new GitHub Actions workflow, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range comments (2)
README.md (2)
Line range hint
17-42: Enhance implementation details section.The "How it works" section needs more detailed information about:
- Error handling mechanisms
- Retry strategies for failed downloads
- Specific scheduling details (timezone, retry policies)
- Data validation processes
Also, convert bare URLs to proper markdown links:
-https://home.treasury.gov/resource-center/data-chart-center/interest-rates/TextView?type=daily_treasury_yield_curve&field_tdr_date_value=2024 +[Treasury Yield Curve Data](https://home.treasury.gov/resource-center/data-chart-center/interest-rates/TextView?type=daily_treasury_yield_curve&field_tdr_date_value=2024)🧰 Tools
🪛 Markdownlint (0.35.0)
26-26: null
Bare URL used(MD034, no-bare-urls)
30-30: null
Bare URL used(MD034, no-bare-urls)
Line range hint
70-89: Enhance sample data documentation.The sample data is helpful, but could be improved by:
- Adding column descriptions
- Explaining data types and formats
- Including validation rules or constraints
🧰 Tools
🪛 Markdownlint (0.35.0)
64-64: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
46-46: null
Bare URL used(MD034, no-bare-urls)
47-47: null
Bare URL used(MD034, no-bare-urls)
48-48: null
Bare URL used(MD034, no-bare-urls)
49-49: null
Bare URL used(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (6)
data/2021-daily-treasury-rates.pklis excluded by!**/*.pkldata/2022-daily-treasury-rates.pklis excluded by!**/*.pkldata/2023-daily-treasury-rates.pklis excluded by!**/*.pkldata/2024-daily-treasury-rates.csvis excluded by!**/*.csvdata/2024-daily-treasury-rates.pklis excluded by!**/*.pkldata/daily-treasury-rates.pklis excluded by!**/*.pkl
📒 Files selected for processing (4)
.github/workflows/build.yml(1 hunks).gitignore(1 hunks)README.md(3 hunks)main.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.0)
main.py
30-30: Missing return type annotation for public function save_and_update_github
Add return type annotation: None
(ANN201)
30-30: Missing type annotation for function argument file_path
(ANN001)
30-30: Missing type annotation for function argument repo
(ANN001)
30-30: Missing type annotation for function argument github_path
(ANN001)
35-35: Logging statement uses f-string
(G004)
44-44: Logging statement uses f-string
(G004)
51-51: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
51-51: Logging statement uses f-string
(G004)
51-51: Use explicit conversion flag
Replace with conversion flag
(RUF010)
52-52: Use raise without specifying exception name
Remove exception name
(TRY201)
83-83: sort_file_names may be undefined, or defined from star imports
(F405)
84-84: merge_files may be undefined, or defined from star imports
(F405)
106-106: f-string without any placeholders
Remove extraneous f prefix
(F541)
106-106: Logging statement uses f-string
(G004)
🪛 Markdownlint (0.35.0)
README.md
26-26: null
Bare URL used
(MD034, no-bare-urls)
30-30: null
Bare URL used
(MD034, no-bare-urls)
64-64: null
Multiple top-level headings in the same document
(MD025, single-title, single-h1)
46-46: null
Bare URL used
(MD034, no-bare-urls)
47-47: null
Bare URL used
(MD034, no-bare-urls)
48-48: null
Bare URL used
(MD034, no-bare-urls)
49-49: null
Bare URL used
(MD034, no-bare-urls)
93-93: null
Bare URL used
(MD034, no-bare-urls)
🪛 actionlint (1.7.4)
.github/workflows/build.yml
17-17: property "deploy" is not defined in object type {auth: {conclusion: string; outcome: string; outputs: {access_token: string; auth_token: string; credentials_file_path: string; id_token: string; project_id: string}}}
(expression)
🪛 yamllint (1.35.1)
.github/workflows/build.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 20-20: wrong indentation: expected 6 but found 4
(indentation)
🔇 Additional comments (3)
.gitignore (1)
164-164: LGTM!
Adding .idea/ to .gitignore is a good practice as it excludes IDE-specific files.
README.md (2)
3-15: LGTM! Well-structured project overview.
The project overview section effectively communicates the purpose and core functionalities of the project.
43-49: 🛠️ Refactor suggestion
Verify external repository references and improve formatting.
Please verify if it's appropriate to reference these external repositories. Also, convert bare URLs to properly formatted markdown links with descriptive text.
✅ Verification successful
Convert bare URLs to formatted markdown links and keep public repository references
The referenced QuantConnect repository is public and appears to be used only as a reference dataset source. Please update the links using proper markdown formatting:
## Reference dataset: QC
- [US Treasury Yield Curve Data](https://www.quantconnect.com/data/us-treasury-yield-curve)
- [Treasury Yield Curve Algorithm](https://github.com/QuantConnect/Lean.DataSource.USTreasury/blob/master/USTreasuryYieldCurveDataAlgorithm.cs)
- [Treasury Data Source Repository](https://github.com/QuantConnect/Lean.DataSource.USTreasury/)
- [Treasury Yield Curve Rate Model](https://github.com/QuantConnect/Lean.DataSource.USTreasury/blob/master/USTreasuryYieldCurveRate.cs)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the referenced repositories are public
for repo in "QuantConnect/Lean.DataSource.USTreasury"; do
gh repo view "$repo" --json isPrivate | jq -r '.isPrivate'
done
Length of output: 152
Script:
#!/bin/bash
# Let's check if there are any sensitive tokens, credentials, or internal references in the codebase
rg -i "(api[_-]?key|secret|token|password|credential|internal)" -g "!{.git,node_modules,dist,build}/*"
# Also check for any references to QuantConnect in the codebase to understand the relationship
rg -i "quantconnect" -g "!{.git,node_modules,dist,build}/*"
Length of output: 607
🧰 Tools
🪛 Markdownlint (0.35.0)
46-46: null
Bare URL used
(MD034, no-bare-urls)
47-47: null
Bare URL used
(MD034, no-bare-urls)
48-48: null
Bare URL used
(MD034, no-bare-urls)
49-49: null
Bare URL used
(MD034, no-bare-urls)
| environment: | ||
| name: production | ||
| url: ${{ steps.deploy.outputs.url }} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add protection rules for production environment
The production environment lacks protection rules. Consider adding:
- Required reviewers
- Deployment approval gates
- Wait timer
Would you like me to provide an example of environment protection configuration?
🧰 Tools
🪛 actionlint (1.7.4)
17-17: property "deploy" is not defined in object type {auth: {conclusion: string; outcome: string; outputs: {access_token: string; auth_token: string; credentials_file_path: string; id_token: string; project_id: string}}}
(expression)
| - name: Build Docker image | ||
| run: | | ||
| docker build -t gcr.io/dfg-analytics-insights-prod/us-department-treasury:latest . | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add .dockerignore file to prevent secrets leakage
The Docker build context includes all files. Create a .dockerignore file to exclude sensitive files (e.g., .env, credentials) from the build context.
Would you like me to generate a .dockerignore file with recommended exclusions?
There was a problem hiding this comment.
Yes, generate a .dockerignore
There was a problem hiding this comment.
@kevinostoll-dfg, here's a recommended .dockerignore file to prevent sensitive and unnecessary files from being included in your Docker build context:
# Python cache files
__pycache__/
*.py[cod]
*$py.class
# Virtual environment directories
env/
venv/
.venv/
# System files
.DS_Store
# Git files
.git
.gitignore
# IDE settings
.idea/
.vscode/
*.sublime-project
*.sublime-workspace
# Environment and secret files
.env
*.env
secrets/
credentials.json
# Data files
*.csv
*.pickle
data/
# Logs
logs/
*.log
# Other
*.bak
*.swp
This will help ensure that sensitive information and unnecessary files are excluded from the build context. Let me know if you'd like to customize it further.
|
|
||
| - name: Build Docker image | ||
| run: | | ||
| docker build -t gcr.io/dfg-analytics-insights-prod/us-department-treasury:latest . |
There was a problem hiding this comment.
Add explicit version tag for Docker image
Using latest tag is considered an anti-pattern as it makes deployments unpredictable and harder to rollback. Consider using a deterministic tag based on commit SHA or release version.
-docker build -t gcr.io/dfg-analytics-insights-prod/us-department-treasury:latest .
+docker build -t gcr.io/dfg-analytics-insights-prod/us-department-treasury:${{ github.sha }} .📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker build -t gcr.io/dfg-analytics-insights-prod/us-department-treasury:latest . | |
| docker build -t gcr.io/dfg-analytics-insights-prod/us-department-treasury:${{ github.sha }} . |
| file_names = sort_file_names(csv_dir) | ||
| merged = merge_files(file_names, csv_dir) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid star imports
The functions sort_file_names and merge_files are imported via star import. This makes it harder to track dependencies and can lead to naming conflicts.
-from udt.utils import *
+from udt.utils import sort_file_names, merge_filesCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.8.0)
83-83: sort_file_names may be undefined, or defined from star imports
(F405)
84-84: merge_files may be undefined, or defined from star imports
(F405)
| For: https://home.treasury.gov/resource-center/data-chart-center/interest-rates/TextView?type=daily_treasury_yield_curve&field_tdr_date_value=2024 | ||
|
|
||
|  |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve screenshot section formatting.
- Convert the bare URL to a proper markdown link
- Use relative path for the image instead of hardcoding the branch
-For: https://home.treasury.gov/resource-center/data-chart-center/interest-rates/TextView?type=daily_treasury_yield_curve&field_tdr_date_value=2024
-
-
+[Data Source](https://home.treasury.gov/resource-center/data-chart-center/interest-rates/TextView?type=daily_treasury_yield_curve&field_tdr_date_value=2024)
+
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| For: https://home.treasury.gov/resource-center/data-chart-center/interest-rates/TextView?type=daily_treasury_yield_curve&field_tdr_date_value=2024 | |
|  | |
| [Data Source](https://home.treasury.gov/resource-center/data-chart-center/interest-rates/TextView?type=daily_treasury_yield_curve&field_tdr_date_value=2024) | |
|  |
🧰 Tools
🪛 Markdownlint (0.35.0)
93-93: null
Bare URL used
(MD034, no-bare-urls)
95-95: null
Files should end with a single newline character
(MD047, single-trailing-newline)
…t-treasury
Summary by CodeRabbit
README.mdfor improved clarity and structure, including new sections on project purpose, functionalities, and sample data..gitignoreto include the.idea/directory in the ignore list.