Refactored jobs to use bash instead of python for portability#186
Refactored jobs to use bash instead of python for portability#186sfreeman422 merged 4 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the cron-style jobs in packages/jobs to run as standalone bash scripts, removing the previous Python/Pipenv implementations and documenting the new runtime dependencies.
Changes:
- Replaced Python job runners with bash scripts for
health-job,fun-fact-job, andpricing-job. - Removed Pipenv/Python artifacts (
*.py,Pipfile,Pipfile.lock) from those job packages. - Updated
README.mdwith scheduled job notes and dependency requirements.
Reviewed changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/jobs/pricing-job/script.sh | New bash implementation for pricing updates via MySQL CLI. |
| packages/jobs/pricing-job/pricing-job.py | Removed Python implementation. |
| packages/jobs/pricing-job/Pipfile.lock | Removed Pipenv lockfile. |
| packages/jobs/pricing-job/Pipfile | Removed Pipenv manifest. |
| packages/jobs/health-job/script.sh | New bash health check + Slack alert implementation using curl. |
| packages/jobs/health-job/job.py | Removed Python implementation. |
| packages/jobs/health-job/Pipfile.lock | Removed Pipenv lockfile. |
| packages/jobs/health-job/Pipfile | Removed Pipenv manifest. |
| packages/jobs/fun-fact-job/script.sh | New bash implementation using curl/jq/mysql + Slack blocks. |
| packages/jobs/fun-fact-job/fun-fact-job.py | Removed Python implementation. |
| packages/jobs/fun-fact-job/Pipfile.lock | Removed Pipenv lockfile. |
| packages/jobs/fun-fact-job/Pipfile | Removed Pipenv manifest. |
| README.md | Added documentation for scheduled jobs and their dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| response_file=$(mktemp) | ||
| payload=$(jq -n \ | ||
| --arg channel "${SLACK_CHANNEL}" \ | ||
| --arg text "${SLACK_TEXT_FALLBACK}" \ | ||
| --argjson blocks "${blocks_json}" \ | ||
| '{channel: $channel, text: $text, blocks: $blocks}') | ||
|
|
||
| response_code=$(curl \ | ||
| --silent \ | ||
| --show-error \ | ||
| --output "${response_file}" \ | ||
| --write-out '%{http_code}' \ | ||
| --request POST \ | ||
| --header "Authorization: Bearer ${MUZZLE_BOT_TOKEN}" \ | ||
| --header 'Content-Type: application/json; charset=utf-8' \ | ||
| --data "${payload}" \ | ||
| https://slack.com/api/chat.postMessage) | ||
|
|
There was a problem hiding this comment.
Similar to the health job: response_file=$(mktemp) is created before response_code=$(curl ...). With set -e, any curl failure (network, TLS, etc.) will exit the script immediately and skip cleanup of the temp file. Consider trapping cleanup or making the curl failure path remove the temp file before returning/aborting.
| for team_id in "${teams[@]}"; do | ||
| [[ -n "${team_id}" ]] || continue | ||
|
|
||
| for item_row in "${items[@]}"; do | ||
| IFS=$'\t' read -r item_id price_pct <<<"${item_row}" | ||
| price=$(awk -v median="${median_rep}" -v pct="${price_pct}" 'BEGIN { printf "%.15f", median * pct }') | ||
| mysql_query "INSERT INTO price(itemId, teamId, price, itemIdId) VALUES(${item_id}, '$(sql_escape "${team_id}")', ${price}, ${item_id});" >/dev/null | ||
| done |
There was a problem hiding this comment.
mysql_query spawns a new mysql process for every item/team insert inside the nested loops, which can be very slow (and puts unnecessary load on the DB) when the number of teams/items grows. Consider batching inserts (e.g., generate SQL and run a single mysql invocation, ideally wrapped in a transaction).
| PATH=/usr/local/bin:/usr/bin:/bin:${PATH:-} | ||
|
|
||
| HEALTH_URL="${HEALTH_URL:-http://127.0.0.1:3000/health}" | ||
| SLACK_CHANNEL="${SLACK_CHANNEL:-#muzzlefeedback}" | ||
| SLACK_MESSAGE=':this-is-fine: `Moonbeam is experiencing some technical difficulties at the moment.` :this-is-fine:' | ||
| MAX_ATTEMPTS="${MAX_ATTEMPTS:-5}" | ||
| SLEEP_SECONDS="${SLEEP_SECONDS:-1}" | ||
| CONNECT_TIMEOUT="${CONNECT_TIMEOUT:-5}" | ||
| MAX_TIME="${MAX_TIME:-15}" | ||
|
|
||
| send_slack_message() { |
There was a problem hiding this comment.
This script relies on curl, grep, and mktemp but never validates they exist, so failures will be less actionable (e.g., curl: command not found). Consider adding require_command checks (similar to other jobs) so missing dependencies fail fast with a clear message.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot fix code review feedback |
|
@sfreeman422 I've opened a new pull request, #187, to work on those changes. Once the pull request is ready, I'll request review from you. |
…tched MySQL inserts (#187) * Initial plan * Fix code review feedback: require_command checks, temp file cleanup, batch MySQL inserts Co-authored-by: sfreeman422 <16405652+sfreeman422@users.noreply.github.com> Agent-Logs-Url: https://github.com/dev-chat/mocker/sessions/1c571688-b133-4945-9edc-a6d11e23df1d --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sfreeman422 <16405652+sfreeman422@users.noreply.github.com>
No description provided.