potiuk edited a comment on pull request #20180: URL: https://github.com/apache/airflow/pull/20180#issuecomment-990572688
> Yeah agree, we do instantiate all provider classes somewhere in our test no?? I can’t remember where but that might be a good place to check this Unfortunately, we only **import** all.the classes :(. Not really instantiate them (that would be impossible as we do not know which parameters to pass to constructors). We (hopefully) instantiate all operators in the unit tests - so there we could (and we plan to for multi-tenancy/db isolation) tap into the db session created and warn if an operator performs an unexpected DB operation. But that is a bit involved because we have to pass through all the 'expected' calls as well to the real db because otherwise many tests will fail. For example if a test performs get_connection() in the execute() - it should be passed through during unit tests. We have the description coming in AIP-44 describing the test harness and what it is going to give us as this is a part of the solution. We will be be able to verify that community managed operators are all compliant with the DB isolation mode (and fix those that aren't). And we can also (i believe) include that check for db operations in init() for the operators as part of the test harness. We can basically check the stack trace and see if any BaseOperator's derived class is calling any db operation in their init. Static checks might also be helpful but they are limited - they can only (reliable) go as far as to check if certain methods (like get_connectiion) are called directly in the _init but any transitive calls will not be possible to track reliably. Only actually instantiating the operators (which can only be done in unit tests IMHO) can give us complete answer. It's also not 100% complete with tests as there might be some combination of parameters in constructors that will choose different,.untested paths, but hopefully unit tests are comprehensive enough to cover those in most cases. Also when we have the harness in place, it will be easy to reproduce and fix any problems with db isolation by adding more unit tests with those paths covered. I am super excited for this part especially - as i wanted to do that check on init'() for quite a while and with multi-tenancy we have finally a good reason to do it. Before that was not justified enough to extend our test harness. -- 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]
