Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SlackAPIFileOperator is not propagating filetype API call #42889

Closed
1 of 2 tasks
hzlmn opened this issue Oct 10, 2024 · 2 comments
Closed
1 of 2 tasks

SlackAPIFileOperator is not propagating filetype API call #42889

hzlmn opened this issue Oct 10, 2024 · 2 comments
Labels
area:providers kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet provider:slack

Comments

@hzlmn
Copy link

hzlmn commented Oct 10, 2024

Apache Airflow Provider(s)

slack

Versions of Apache Airflow Providers

2.9.2

Apache Airflow version

2.9.2

Operating System

MacOS 14.3.1

Deployment

Amazon (AWS) MWAA

Deployment details

No response

What happened

SlackAPIFileOperator not propagating correctly filetype ending up sending csv file and json as plain text uploads.

As you can see here filetype is passed to initial params but not used than in execution.
https://github.com/apache/airflow/blob/main/providers/src/airflow/providers/slack/operators/slack.py#L263-L271

What you think should happen instead

Should correctly pass down filetype value.

Can be fixed as following

class SlackAPIFileOperator(SlackAPIOperator):
    ...
    def execute(self, context: Context):
        self._method_resolver(
            channels=self.channels,
            # For historical reason SlackAPIFileOperator use filename as reference to file
            file=self.filename,
            filetype=self.filetype,   # here missing filetype
            content=self.content,
            initial_comment=self.initial_comment,
            title=self.title,
        )

How to reproduce

slack = SlackAPIFileOperator(
    task_id="slack_file_upload",
    dag=dag,
    slack_conn_id="slack",
    channel="#general",
    initial_comment="Hello World!",
    filename="hello_world.csv",
    filetype="csv",
    content="hello,world,csv,file",
)

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@hzlmn hzlmn added area:providers kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Oct 10, 2024
Copy link

boring-cyborg bot commented Oct 10, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@Bowrna
Copy link
Contributor

Bowrna commented Oct 16, 2024

@hzlmn On debugging further with the data you shared above, could you tell me what method_version you are using to upload files using SlackOperator? The default one is "v2", while you can still pass the old version "v1".

title: str | None = None,
method_version: Literal["v1", "v2"] = "v2",
channel: str | Sequence[str] | None | ArgNotSet = NOTSET,

Here the execute method to invoke the Slack API chooses the upload based on the method version.

@property
def _method_resolver(self):
if self.method_version == "v1":
return self.hook.send_file
return self.hook.send_file_v1_to_v2
def execute(self, context: Context):
self._method_resolver(
channels=self.channels,
# For historical reason SlackAPIFileOperator use filename as reference to file
file=self.filename,
content=self.content,
initial_comment=self.initial_comment,
title=self.title,
)

The send_file method call of v1 function invokes API call where we send the filetype. The above issue fix that you have recommended applies here.

Then there is another version v2 which is the updated version, where we upload file with different SDK method call. We pass a dict of file_uploads. ( in this dict we can pass file(or content) (here file represents the filepath, content represents the actual filecontent), filename, title, snippet_type, alt_txt)

def send_file_v1_to_v2(
self,
*,
channels: str | Sequence[str] | None = None,
file: str | Path | None = None,
content: str | None = None,
filename: str | None = None,
initial_comment: str | None = None,
title: str | None = None,
filetype: str | None = None,
) -> list[SlackResponse]:
"""
Smooth transition between ``send_file`` and ``send_file_v2`` methods.
:param channels: Comma-separated list of channel names or IDs where the file will be shared.
If omitting this parameter, then file will send to workspace.
File would be uploaded for each channel individually.
:param file: Path to file which need to be sent.
:param content: File contents. If omitting this parameter, you must provide a file.
:param filename: Displayed filename.
:param initial_comment: The message text introducing the file in specified ``channels``.
:param title: Title of the file.
:param filetype: A file type identifier.
"""
if not exactly_one(file, content):
raise ValueError("Either `file` or `content` must be provided, not both.")
if file:
file = Path(file)
file_uploads: FileUploadTypeDef = {"file": file.__fspath__(), "filename": filename or file.name}
else:
file_uploads = {"content": content, "filename": filename}
file_uploads.update({"title": title, "snippet_type": filetype})

In the above method we are passing the filetype into snippet_type. But snippet_type is an optional argument for syntax highlighting in case code content is uploaded. ( It can support types like python, HTML etc. Passing the filetype into snippet_type seems to be incorrect). In V2 version, there is no explicit way to pass the filetype, as I think with file upload directly as multipart binary, the metadata or MIME of the file content can be used to find the file type.

@potiuk Can you tell me if I can raise a fix where we can get another param for snippet_type, rather than passing the filetype into it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet provider:slack
Projects
None yet
Development

No branches or pull requests

3 participants