Re-enabling uploading job metrics step to redshift #2363
Conversation
|
@ScottTodd @catan2001 Issue was seen as the _log function in github_actions_utils.py was appending warning and log messages to the output while populate_redshift_db.py script was looking for a json file to do the processing and upload the job results to redshift. With these changes stderr ends up in log annotation and doesn’t corrupt JSON. Passing workflow : https://github.com/ROCm/TheRock/actions/runs/19829505982/job/56831838522 PR: #2363 |
ScottTodd
left a comment
ScottTodd
left a comment
There was a problem hiding this comment.
Is this "redshift database" feature still useful? The integration into this one workflow was a proof of concept. I would rather delete the code if unused and focus our time and energy on solutions that scale to all of our workflows and build processes (see also recent discussion on #2356 (comment))
| python3 build_tools/github_actions/fetch_job_status.py > job_summary.json | ||
| echo "Job summary written to job_summary.json" | ||
| echo "Contents:" | ||
| cat job_summary.json | ||
| JOB_SUMMARY=$(cat job_summary.json) |
There was a problem hiding this comment.
This is very roundabout - just have the script write what it needs directly to the file and set any variables as needed.
| --redshift-cluster-endpoint "${{ secrets.THEROCK_REDSHIFT_CLUSTER_ENDPOINT }}" \ | ||
| --redshift-username "${{ secrets.THEROCK_REDSHIFT_CLUSTER_USERNAME }}"\ | ||
| --dbname "${{ secrets.THEROCK_REDSHIFT_CLUSTER_DBNAME }}" \ | ||
| --redshift-password "${{ secrets.THEROCK_REDSHIFT_CLUSTER_PASSWORD }}" \ | ||
| --redshift-port "${{ secrets.THEROCK_REDSHIFT_CLUSTER_ACCESS_PORT }}" \ |
There was a problem hiding this comment.
We should delete these secrets and find another authentication method that is more secure.
| def _log(*args, **kwargs): | ||
| print(*args, **kwargs) | ||
| sys.stdout.flush() | ||
| print(*args, **kwargs, file=sys.stderr) | ||
| sys.stderr.flush() |
There was a problem hiding this comment.
No changes should be needed here if the script were to just write that it needs to files directly instead of relying on stdout piping.
The redshift DB is currently under investigation from @geomin12 to be used for some more telemetry work that he is working on too, so in my opinion this might be useful to have it back up and running. I can working on implementing the changes requested in #2363 (comment) . This prresently gives us more than just this build linux packaged workflow details. The github api is able to pull all details for a particular CI workflow run which encompasses the test details too in CI nightly runs. |
Motivation
This PR is intended to fix the json loads issue we were seeing with the uploading job metrics step to redshift
Technical Details
Issue was seen as the _log function in github_actions_utils.py was appending warning and log messages to the output while populate_redshift_db.py script was looking for a json file to do the processing and upload the job results to redshift.
With these changes stderr ends up in log annotation and doesn’t corrupt JSON.
Test Plan
Passing workflow run with changes: https://github.com/ROCm/TheRock/actions/runs/19829505982/job/56831838522
Test Result
Passing workflow run with changes: https://github.com/ROCm/TheRock/actions/runs/19829505982/job/56831838522
Submission Checklist