SameerMesiah97 commented on PR #62378:
URL: https://github.com/apache/airflow/pull/62378#issuecomment-3952986463

   > Thank you @SameerMesiah97 for @bugraoz93 for your feedback and helping me 
get this PR in a much better state. I'm continuing to work on this.
   > 
   > I also realize that the `SnowflakeSqlApiHook` has a separate private key 
flow, so I'll add support to that. I'm also wondering if those should be 
consolidated to be more DRY, and whether that would be out of scope. 🤔
   
   If you wish to do that, I would broaden the scope for this issue to include 
`SnowflakeSqlApiHook` as well (update the description). And I think we should 
see how the private key flow looks in `SnowflakeSqlApiHook` before considering 
consolidating in say the `utils` module for the provider. Note that 
`SnowflakeSqlApiHook` inherits from `SnowflakeHook`and whenever it needs to use 
functionality from there, it usually either calls the method via self (see 
`_get_conn_params`) or uses `super() `on the relevant method alongside any 
injected code (see `get_oauth_token`). I think should wait to see how your 
implementation for `SnowflakeSqlApiHook` looks like to see how it should be 
handled.


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