uranusjr commented on code in PR #73457: URL: https://github.com/apache/airflow/pull/73457#discussion_r4132413678
########## airflow-core/adr/dag-processing/0001-dag-importer-process-model.md: ########## @@ -0,0 +1,374 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + --> + +# ADR-0001: Dag Importer Process Model — Who Owns the Parse Process + +## Status + +Proposed + +## Context + +[AIP-85](https://cwiki.apache.org/confluence/x/_Q7OEg) adds `AbstractDagImporter` +(`task-sdk/src/airflow/sdk/importers/`) so the Dag processor can parse sources other than Python. +The interface as it stands says what importing *means* for a format — `list_dag_definitions`, +`import_definition`, `get_source_code` — and says nothing about *where* the import runs. + +Where it runs is still decided by the Dag processor manager, which forks one +`DagFileProcessorProcess` per file and runs a Python parse in it. That is the wrong shape for both +directions the AIP opens: + +- A format that only has to be **read** — JSON, YAML, a manifest naming Dags — pays for a process + it does not need, on every parse of every file. +- A format backed by **another runtime** needs a *different* process — a JVM, a Go binary — not a + Python fork. Nothing in the current model can express that, which is why Dag parsing was cut + from AIP-108's scope and [ADR-0004](../lang-sdk/0004-dag-parsing.md) was left "retained for when + AIP-85 is revisited". + +An earlier proposal filled the gap by inserting a layer between the manager and the parse process, +mirroring the executor's worker pool: the manager forks a generic parse worker, which then +dispatches to an importer. This ADR settles the process model without that layer, and fixes the +handful of interface obligations that follow from it. + +Terms: a **definition** is the smallest thing an importer can import on its own. For Python that is +a module; for a zip archive it is a member, not the archive. One definition may yield several Dags. + +## Decision + +### 1. The importer owns the process, and nothing sits between it and the manager + +``` +today + + manager loop + │ one queue entry per FILE + ▼ + fork DagFileProcessorProcess ← always a Python fork, whatever the format + └── _parse_file → DagBag(file) → every Dag in the file, one sys.modules + +decided + + manager loop + │ one queue entry per DEFINITION + ▼ + importer.start_import(definition, bundle, context=...) → ImportHandle + ├── read it here, already finished static format — no process at all + ├── fork_import() → parse child Python — one child per definition + └── hand to a runtime it keeps warm Java / Go — the importer's own process +``` + +`start_import` is the extension point for *how the importer's language runs*; `import_definition` +remains the single statement of what importing means, and both the inline path and the child end up +back in it, so there is never a second implementation of the import itself. + +The default `start_import` imports inline and returns an already-finished handle. A static format Review Comment: Both the function name (“start” import) and the below diagram (around line 101) seems to suggest the function is actually designed to not be blocking. And practical implementations should all be non-blocking anyway. Maybe the we should simply not have a default implementation at all? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
