fix: set base_log_folder to display task logs in the UI - #834
Conversation
|
@sbernauer I'm not the best person to review this as I co-authored it |
razvan
left a comment
There was a problem hiding this comment.
lgtm with two small suggestions
| 'base_log_folder': '{log_dir}', | ||
| # `serve_logs` on the workers serves task logs from this directory, so it must be | ||
| # the folder the Task SDK writes task logs to, not the Vector agent log directory. | ||
| 'base_log_folder': os.path.expanduser(conf.get('logging', 'BASE_LOG_FOLDER')), |
There was a problem hiding this comment.
Claude suggests to use conf.get_mandarory_value() as it would produce a better error message
| 'base_log_folder': os.path.expanduser(conf.get('logging', 'BASE_LOG_FOLDER')), | |
| 'base_log_folder': os.path.expanduser(conf.get_mandarory_value('logging', 'BASE_LOG_FOLDER')), |
There was a problem hiding this comment.
That should be mandatory: somewhere a typo slipped in
There was a problem hiding this comment.
why aren't you acquainted with the new mandarory technology? it's when you spill fruit juice over your keyboard so that your fingers stick closer to the keys.
There was a problem hiding this comment.
mandalorian-ory technology? Now we're talking
|
|
||
| use super::*; | ||
|
|
||
| fn resolved_image(product_version: &str) -> ResolvedProductImage { |
There was a problem hiding this comment.
There is already a function called resolved_product_image_stub() in this module that could either be removed or reused.
Description
The Airflow UI should be able to show task logs which are requested from the worker pod. This requires setting
BASE_LOG_FOLDERto the correct place, which differs from our log mount - this PR fixes that.Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker