Enhance SqlToSlackOperator to support attachments

[JeffryMAC]

Use case / motivation
following to #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

airflow/airflow/providers/slack/operators/slack.py

Line 159 in fe59f26

class SlackAPIFileOperator(SlackAPIOperator):

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.

airflow/airflow/providers/slack/hooks/slack.py

Line 85 in d404cb0

def call(self, api_method: str, *args, **kwargs) -> None:

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.

airflow/airflow/providers/slack/operators/slack.py

Line 159 in fe59f26

class SlackAPIFileOperator(SlackAPIOperator):

@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 :)

[JeffryMAC]

@kishvanchee did you had time to work on it?

[eladkal]

@kishvanchee due to passed time I'm unassigning you.
This issue is available for anyone who wants to pick it

[eladkal]

Update:
Now that #24663 is merged. The feature should be implemented in SqlToSlackOperator

[kevgeo]

Hi @Taragolis, are you still working on adding an enhancement. Could I take up this issue?

[Taragolis]

Hi @kevgeo I have a hectic weeks, so feel free to take this issue, you could grab some my previous changes if you find some useful parts.

I have a plan to make changes in Slack hooks first however I really do not know when I have a time for that.

[kevgeo]

Thanks @Taragolis, I will look into your previous PR too.