AP-783: Add GOBI processing DAG - #96
Conversation
d27119e to
5d8db27
Compare
anarchivist
changed the title
davezuckerman
left a comment
davezuckerman
left a comment
There was a problem hiding this comment.
I'm not versed enough in Python to suggest any potential improvements. As far as the logic goes this looks completely accurate and should do everything the current Gobi script is doing.
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
Not too much to change, but I think it warrants a bit of fine-tuning before merge.
| final_path, | ||
| ) | ||
| self.skipped_providers.add(provider) | ||
| temporary_path.unlink() |
There was a problem hiding this comment.
Do we want to remove this if there's already an output, or do we want to keep it for debugging purposes? I'm on the fence; this shouldn't really happen in practice so it's probably fine to leave the file, but we also don't want to fill up the disk with a bunch of temporary nonsense files that don't need to be there if something goes sideways.
There was a problem hiding this comment.
i'm thinking we might want to retain this for debugging. however, i can see that the use of the UUID-based directory here probably means we need to change the temporary path construction to use something along the lines of run_dir() as used in other Dags if we want to support a shared set of cleanup processes. what do you think?
There was a problem hiding this comment.
Using something like run_dir makes sense, with a base_dir somewhere inside the GOBI storage path (perhaps the output directory).
Co-authored-by: Codex GPT-5.6-sol <noreply@openai.com>
Co-authored-by: Codex GPT-5.6-sol <noreply@openai.com>
Co-authored-by: Codex GPT-5.6-sol <noreply@openai.com>
Co-authored-by: Codex GPT-5.6-sol <noreply@openai.com>
Use run-scoped staging directories on the output filesystem so abandoned files can be attributed and cleaned up without weakening atomic publication. Co-authored-by: Codex GPT-5.6-sol <noreply@openai.com>
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
r+ I see no issues, but have not done a local test run yet.
4 participants
Migrates the existing GOBI processing script to Airflow using Codex.