Airflow: Enhance SnowflakeToSlackOperator to support attachments

[JeffryMAC]

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.

[kishvanchee]

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

[JeffryMAC]

@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

[kishvanchee]

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

[JeffryMAC]

worth asking @simond as the author of the original PR and @feluelle @turbaszek who approved it.

[simond]

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!

[kishvanchee]

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

[turbaszek]

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 👍

[fatmumuhomer]

@turbaszek I can work on this one for the workshop.

[feluelle]

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.

[kishvanchee]

@feluelle I am, I didn't get this assigned to me though. Although now I am confused whether I should be working on this...

[fatmumuhomer]

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

[feluelle]

Thanks @fatmumuhomer :)