potiuk commented on PR #27854: URL: https://github.com/apache/airflow/pull/27854#issuecomment-1324866930
cc: @vincbeck @kazanzhy @eladkal I think we need to re-review the original change with even more scrutiny (I am going to do that shortly). While looking at the Databricks problem i found that we missed more than `schema` - `process_output` still used schema as parameter even if the SQLExecuteQueryOperator does not pass it. Also there was the backwards compatibility problem in Redshift: https://github.com/apache/airflow/pull/27602 indicates that there might be more problems that we might need to fix. For example, I am not 100% sure if passing of the results is as intended - from what i See DBApiHook can return `-> Any | list[Any] | None` and we are processing the output like that: ``` for out in output: self._process_output(*out) ``` This would only work in case `List` is returned. I think we need to really carefully do the re-review. @alexott - I am going to release "fix" wave as soon as we are confident that there are no more fixes here or in `common.sql` - but I want to do it before the long US weekend (and let it run till Tuesday at least due to this long weekend). I expect because of Thanksgiving, very few people will be available/looking at candidates - but also thanks to Thanksgiving likely very few people will be upgrading providers, so I think we can wait a bit with common.sql 1.3.1 release and spend a bit of time trying to find out if there are more problems (releasing 1.3.1 quickly is IMHO crucial to prevent the #27838 from flooding us with problems so I do not want to delay it for more than a day more) . Hopefully this will help us to mitigates problems related to common.sql in a "smoth" way. -- 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]
