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

   I attempted to replace all the contrib classes with PEP-562 ones. Looking 
forward to comments. 
   I left the tool that I used to generate "__deprecated_classas" dictionaries 
as there are quite a number of non-contrib deprecations we could remove as next 
step, so it might be handy. But I did not want to apply it just yet to the 
other files - they need a bit more selective approach, so I wanted to make sure 
the approach is cool.
   
   Also the presence of the tool might be good as part of review - to see that 
I have not missed anything. This change is -10K lines, so likely easier to 
review the "method", than the result. All the __deprecated_classes were 
generated using this tool.
   
   Few things I would like to get comments on (It's the first time I apply 
PEP-562): 
   
   * I only dynamically generate modules and classes in the modules. I left 
packages in "contrib" folders as they were. Likely it would be possible to also 
dynamically generate packages - but this is beyond PEP-562 andI think this is a 
good balance between number of files/lines left and "explicitness" of the 
solution. But if others have comments / experience here to generate also 
packages, I woudl love to hear it.
   
   * I had to remove all the tests for the classes moved to 
__deprecated_classes. The reason is that our tests attempt to reload the 
modules, and fail with those errors:
   ```
   ModuleNotFoundError: spec not found for the module 
'airflow.contrib.operators.s3_to_gcs_operator'
   ```
   And if you think of it - it makes sense. We generated the modules out of 
thin air, so they do not have spec to use to  reload them. And those tests are 
I think quite a bit redundant now - since we know the warnings are generated by 
``__getattr__`` and we can test it manually (I did), there is no reason it 
should fail for the others. So keeping extra tests for those seems quite 
redundant.
   


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