potiuk commented on PR #68511:
URL: https://github.com/apache/airflow/pull/68511#issuecomment-5172468247

   Reworked the mechanism off the back of this round of review — the approach 
changed, so a summary rather than a diff description.
   
   The prebuilt allow-list is gone. Resolution now splits the stored name, 
looks the module up in `sys.modules`, and reads the class out of that module's 
namespace, requiring the result to be an `AirflowException` subclass. No 
import, no cache, no map.
   
   That drops three separate problems the map had:
   
   - **It could not be complete.** Built once from the subclass tree, it 
permanently rejected any subclass registered later — a provider or plugin 
loaded lazily. Not "blocked at launch", blocked for the life of the process.
   - **It broke old blobs.** Keying on `cls.__module__` cannot follow a 
re-export. These exceptions moved to `airflow.sdk.exceptions` in 3.2.0, so the 
map only held `airflow.sdk.exceptions.AirflowException` while every blob 
written by 3.0/3.1 stores `airflow.exceptions.AirflowException`. 
`import_string` followed the re-export; the map could not. Upgrading would have 
stranded stored blobs carrying an exception node.
   - **It needed invalidation.** With no map there is nothing to go stale.
   
   The property the change exists for is unchanged: nothing in the stored blob 
can cause an import. A module that is not already loaded does not resolve, and 
the class is read from `vars(module)` rather than with `getattr`, so a 
module-level `__getattr__` — which Airflow uses for deprecation shims and lazy 
provider re-exports, some of which import on access — stays out of the path. 
`subprocess.check_output`, `os.system` and `builtins.eval` are all still 
refused with their modules loaded.
   
   This also answers the concern raised earlier in this thread about 
lazily-imported plugin exceptions: an allow-list built at launch was the wrong 
shape for that, and there no longer is one.
   
   ---
   Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
   


-- 
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]

Reply via email to