You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/06/04 20:19:32 UTC
[GitHub] [airflow] JeffryMAC opened a new issue #9145: Enhance SnowflakeToSlackOperator to support attachments
JeffryMAC opened a new issue #9145:
URL: https://github.com/apache/airflow/issues/9145
**Use case / motivation**
following to https://github.com/apache/airflow/pull/9023
Current implementation of SnowflakeToSlackOperator adds the data frame result to the slack message. That way it's not suitable for large data. Even 50 rows is too much. In some cases it's better to send the message with attachment as csv file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-704875261
Is this taken? Can I work on this?
I feel the `SlackAPIFileOperator` https://github.com/apache/airflow/blob/fe59f2622337616fe1902bb6d7e1bce6f002ffa8/airflow/providers/slack/operators/slack.py#L159 might be used here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] simond commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
simond commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-706330446
Oof, I can't really remember the reason I used the `SlackWebHook` over `SlackHook` unfortunately. I think it was because it is simpler to use for basic messages. Now that you want to add an attachment too then yea, you may have to move to the SlackHook instead unfortunately!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JeffryMAC commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
JeffryMAC commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-762668605
@kishvanchee did you had time to work on it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-706335160
Ah okie. Does it make more sense to use `SlackHook` now just for the attachment part? Or should I try refactoring the current implementation with `SlackHook` itself? Not sure about design choices/patterns here.
I would appreciate any help @turbaszek @feluelle
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee edited a comment on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
kishvanchee edited a comment on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-704875261
Is this taken? Can I work on this?
I feel the `SlackAPIFileOperator` https://github.com/apache/airflow/blob/fe59f2622337616fe1902bb6d7e1bce6f002ffa8/airflow/providers/slack/operators/slack.py#L159 might be used here?
I'm thinking something along the lines of storing the dataframe as a csv in `/tmp/` and then pass it through? Not sure if streaming the dataframe as a file is a possible option..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-715257376
@feluelle I am, I didn't get this assigned to me though. Although now I am confused whether I should be working on this...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-706678618
> Does it make more sense to use `SlackHook` now just for the attachment part? Or should I try refactoring the current implementation with `SlackHook` itself?
I think it would be best to reuse what we already have 👍
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee edited a comment on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
kishvanchee edited a comment on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-705167120
Should I be using `SlackHook` only for file uploads while at the moment the operator is using `SlackWebhookHook` ?
Docstring for SlackWebhookHook says
> :param attachments: The attachments to send on Slack. Should be a list of
dictionaries representing Slack attachments.
:type attachments: list
This is confusing to me since I can attach list of dictionaries, but not file objects.
While for SlackHook I can find a suitable call method which actually does the file upload.
https://github.com/apache/airflow/blob/d404cb06dd52d391a9ef26ef700b61f04a771cd5/airflow/providers/slack/hooks/slack.py#L85
[This stackoverflow answer](https://stackoverflow.com/a/36287627/5675288) says that I can't use a webhook, but must use an api for file upload. The WebClient is what is actually being used in the `SlackAPIFileOperator` too. https://github.com/apache/airflow/blob/fe59f2622337616fe1902bb6d7e1bce6f002ffa8/airflow/providers/slack/operators/slack.py#L159
@jeffryMAC any thoughts?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] fatmumuhomer commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
fatmumuhomer commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-714602725
@turbaszek I can work on this one for the workshop.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JeffryMAC commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
JeffryMAC commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-704934197
@kishvanchee it's not taken you can work on this.
storing in tmp is what many transfer operators do
examples:
https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/gcs_to_sftp.py#L164
https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/sql_to_gcs.py#L184
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-800347872
@kishvanchee due to passed time I'm unassigning you.
This issue is available for anyone who wants to pick it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] fatmumuhomer commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
fatmumuhomer commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-715383127
@kishvanchee feel free to take it and work on it if you would like. I was using this as a practice issue in the contributor workshop that @turbaszek was teaching. I'm happy to grab a different issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JeffryMAC commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
JeffryMAC commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-706300177
worth asking @simond as the author of the original PR and @feluelle @turbaszek who approved it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] feluelle commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
feluelle commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-715092967
I thought @kishvanchee liked to work on that one?
I think using SlackHook for the file upload makes sense and the use the SlackWebHook for the rest.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] fatmumuhomer edited a comment on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
fatmumuhomer edited a comment on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-715383127
@kishvanchee please keep the issue and work on it if you would like. I was using this as a practice issue to learn how to contribute in the workshop that @turbaszek was teaching. I'm happy to grab a different issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] feluelle commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
feluelle commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-717788396
Thanks @fatmumuhomer :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-705167120
Should I be using `SlackHook` only for file uploads while at the moment the operator is using `SlackWebhookHook` ?
Docstring for SlackWebhookHook says
> :param attachments: The attachments to send on Slack. Should be a list of
dictionaries representing Slack attachments.
:type attachments: list
This is confusing to me since I can attach list of dictionaries, but not file objects.
While for SlackHook I can find a suitable call method which actually does the file upload.
https://github.com/apache/airflow/blob/d404cb06dd52d391a9ef26ef700b61f04a771cd5/airflow/providers/slack/hooks/slack.py#L85
[This stackoverflow answer](https://stackoverflow.com/a/36287627/5675288) says that I can't use a webhook, but must use an api for file upload. The WebClient is what is actually being used in the `SlackAPIFileOperator` too. https://github.com/apache/airflow/blob/fe59f2622337616fe1902bb6d7e1bce6f002ffa8/airflow/providers/slack/operators/slack.py#L159
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] simond edited a comment on issue #9145: Enhance SnowflakeToSlackOperator to support attachments
Posted by GitBox <gi...@apache.org>.
simond edited a comment on issue #9145:
URL: https://github.com/apache/airflow/issues/9145#issuecomment-706330446
Oof, I can't really remember the reason I used the `SlackWebHook` over `SlackHook` unfortunately. I think it was because it is simpler to use for basic messages. Now that you want to add an attachment too then yea, you may have to move to the SlackHook instead!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org