dwreeves commented on PR #62378:
URL: https://github.com/apache/airflow/pull/62378#issuecomment-3953247376
> I think should wait to see how your implementation for SnowflakeSqlApiHook
looks like to see how it should be handled.
I just pushed the change, so you can see.
I moved `get_private_key()` to `SnowflakeHook`, and gave it a return value
instead of setting `self.private_key`.
If you want true, pure backwards compat, then we can have
`SnowflakeSqlApiHook` have:
```python
def get_private_key(self):
self.private_key = super().get_private_key()
return self.private_key
```
But I'm not sure if that's necessary. `get_private_key()` is essentially a
private method (not marked as such, but end-users would not realistically ever
call it) and it is only called once, so I modified the code at that point to be
`self.private_key = self.get_private_key()` instead of setting
`self.private_key` as a side-effect.
(If you treat `get_private_key()` literally as a public method, this is a
minorly breaking change, but I really have a hard time seeing it as a true
public method, and often people writing Python code aren't so strict about such
decisions so I'm OK interpreting it as private.)
I did not bother to put inside of `utils`. I'm not against it, I have no
opinion and I don't want to go crazy touching everything for code I don't
maintain. As far as I see it, this code is better and DRYer than I left it, and
I see what's here already as an improvement. Let me know if you think it should
go in `utils` and what that would look like. I do see you touch this provider
package quite a bit so you can say better than me.
--
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]
