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 2021/08/03 23:12:50 UTC
[GitHub] [airflow] subkanthi opened a new pull request #17400: Example dag slackfile
subkanthi opened a new pull request #17400:
URL: https://github.com/apache/airflow/pull/17400
closes: #17368
- Fixed bug with SlackAPIFileOperator with uploading file.
- Added example dag to demonstrate support for uploading file and file content.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi commented on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-896229411
> @subkanthi it's probably related because the failing tests are of Slack
>
> ```
> =================================== FAILURES ===================================
> _________ TestSlackAPIFileOperator.test_api_call_params_with_file_args _________
>
> self = <tests.providers.slack.operators.test_slack.TestSlackAPIFileOperator testMethod=test_api_call_params_with_file_args>
> mock_hook = <MagicMock name='SlackHook' id='139893250056768'>
>
> @mock.patch('airflow.providers.slack.operators.slack.SlackHook')
> def test_api_call_params_with_file_args(self, mock_hook):
> test_slack_conn_id = 'test_slack_conn_id'
>
> slack_api_post_operator = SlackAPIFileOperator(
> task_id='slack', slack_conn_id=test_slack_conn_id, file='./test.csv', filetype='csv'
> )
>
> > slack_api_post_operator.execute()
>
> tests/providers/slack/operators/test_slack.py:215:
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>
> self = <Task(SlackAPIFileOperator): slack>, kwargs = {}
> slack = <MagicMock name='SlackHook()' id='139893328790136'>
>
> def execute(self, **kwargs):
> """
> The SlackAPIOperator calls will not fail even if the call is not unsuccessful.
> It should not prevent a DAG from completing in success
> """
> slack = SlackHook(token=self.token, slack_conn_id=self.slack_conn_id)
>
> # If file content is passed.
> if self.content is not None:
> self.api_params = {
> 'channels': self.channel,
> 'content': self.content,
> 'initial_comment': self.initial_comment,
> }
> slack.call(self.method, data=self.api_params)
> # If file name is passed.
> elif self.file is not None:
> self.api_params = {
> 'channels': self.channel,
> 'filename': self.file,
> 'filetype': self.filetype,
> 'initial_comment': self.initial_comment,
> }
> > with open(self.file, "rb") as file_name:
> E FileNotFoundError: [Errno 2] No such file or directory: './test.csv'
>
> airflow/providers/slack/operators/slack.py:242: FileNotFoundError
> ```
Yes thanks I noticed that too, cant reproduce it on my local, still checking..
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r683187834
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -187,10 +187,14 @@ def _read(self, ti, try_number, metadata=None):
response.encoding = "utf-8"
if response.status_code == 403:
- log += "*** !!!! Please make sure that all your webservers and workers have" \
- " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
- log += "*** See more at https://airflow.apache.org/docs/apache-airflow/" \
- "stable/configurations-ref.html#secret-key\n***"
+ log += (
+ "*** !!!! Please make sure that all your webservers and workers have"
+ " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
+ )
+ log += (
+ "*** See more at https://airflow.apache.org/docs/apache-airflow/"
+ "stable/configurations-ref.html#secret-key\n***"
+ )
Review comment:
Needs rebase
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r683187682
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -187,10 +187,14 @@ def _read(self, ti, try_number, metadata=None):
response.encoding = "utf-8"
if response.status_code == 403:
- log += "*** !!!! Please make sure that all your webservers and workers have" \
- " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
- log += "*** See more at https://airflow.apache.org/docs/apache-airflow/" \
- "stable/configurations-ref.html#secret-key\n***"
+ log += (
+ "*** !!!! Please make sure that all your webservers and workers have"
+ " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
+ )
+ log += (
+ "*** See more at https://airflow.apache.org/docs/apache-airflow/"
+ "stable/configurations-ref.html#secret-key\n***"
+ )
Review comment:
I broke it and just fixed it in main (bad Jarek!)
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -187,10 +187,14 @@ def _read(self, ti, try_number, metadata=None):
response.encoding = "utf-8"
if response.status_code == 403:
- log += "*** !!!! Please make sure that all your webservers and workers have" \
- " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
- log += "*** See more at https://airflow.apache.org/docs/apache-airflow/" \
- "stable/configurations-ref.html#secret-key\n***"
+ log += (
+ "*** !!!! Please make sure that all your webservers and workers have"
+ " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
+ )
+ log += (
+ "*** See more at https://airflow.apache.org/docs/apache-airflow/"
+ "stable/configurations-ref.html#secret-key\n***"
+ )
Review comment:
Needs rebase
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi commented on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-899477384
> the file/file_name meaning is reversed here (and add backwards_compatibility change) - they should be swapped
@potiuk can u please check again when u get a chance
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r683186470
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -187,10 +187,14 @@ def _read(self, ti, try_number, metadata=None):
response.encoding = "utf-8"
if response.status_code == 403:
- log += "*** !!!! Please make sure that all your webservers and workers have" \
- " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
- log += "*** See more at https://airflow.apache.org/docs/apache-airflow/" \
- "stable/configurations-ref.html#secret-key\n***"
+ log += (
+ "*** !!!! Please make sure that all your webservers and workers have"
+ " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
+ )
+ log += (
+ "*** See more at https://airflow.apache.org/docs/apache-airflow/"
+ "stable/configurations-ref.html#secret-key\n***"
+ )
Review comment:
This isn't related to this PR
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #17400:
URL: https://github.com/apache/airflow/pull/17400
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r683186470
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -187,10 +187,14 @@ def _read(self, ti, try_number, metadata=None):
response.encoding = "utf-8"
if response.status_code == 403:
- log += "*** !!!! Please make sure that all your webservers and workers have" \
- " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
- log += "*** See more at https://airflow.apache.org/docs/apache-airflow/" \
- "stable/configurations-ref.html#secret-key\n***"
+ log += (
+ "*** !!!! Please make sure that all your webservers and workers have"
+ " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
+ )
+ log += (
+ "*** See more at https://airflow.apache.org/docs/apache-airflow/"
+ "stable/configurations-ref.html#secret-key\n***"
+ )
Review comment:
This isn't related to this PR
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r689136892
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -195,49 +195,50 @@ class SlackAPIFileOperator(SlackAPIOperator):
:type content: str
"""
- template_fields = ('channel', 'initial_comment', 'filename', 'filetype', 'content')
+ template_fields = ('channel', 'initial_comment', 'filetype', 'content')
ui_color = '#44BEDF'
def __init__(
self,
channel: str = '#general',
initial_comment: str = 'No message has been set!',
- filename: str = None,
+ file: str = None,
filetype: str = None,
content: str = None,
**kwargs,
) -> None:
self.method = 'files.upload'
self.channel = channel
self.initial_comment = initial_comment
- self.filename = filename
+ self.file = file
self.filetype = filetype
self.content = content
self.file_params = {}
super().__init__(method=self.method, **kwargs)
- def construct_api_call_params(self) -> Any:
+ def execute(self, **kwargs):
+ """
+ The SlackAPIOperator calls will not fail even if the call is not unsuccessful.
+ It should not prevent a DAG from completing in success
+ """
+ slack = SlackHook(token=self.token, slack_conn_id=self.slack_conn_id)
+
+ # If file content is passed.
if self.content is not None:
self.api_params = {
'channels': self.channel,
'content': self.content,
'initial_comment': self.initial_comment,
}
- elif self.filename is not None:
+ slack.call(self.method, data=self.api_params)
+ # If file name is passed.
+ elif self.file is not None:
self.api_params = {
'channels': self.channel,
- 'filename': self.filename,
+ 'filename': self.file,
'filetype': self.filetype,
'initial_comment': self.initial_comment,
}
- self.file_params = {'file': self.filename}
-
- def execute(self, **kwargs):
- """
- The SlackAPIOperator calls will not fail even if the call is not unsuccessful.
- It should not prevent a DAG from completing in success
- """
- if not self.api_params:
- self.construct_api_call_params()
- slack = SlackHook(token=self.token, slack_conn_id=self.slack_conn_id)
- slack.call(self.method, data=self.api_params, files=self.file_params)
+ with open(self.file, "rb") as file_name:
Review comment:
changed to file_handle, thanks
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-892378608
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r689686836
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -162,15 +161,16 @@ class SlackAPIFileOperator(SlackAPIOperator):
.. code-block:: python
# Send file with filename and filetype
- 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",
- )
+ with open("test.txt", "rb") as file:
Review comment:
What's changing here? `file` variable looks unused.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r689136934
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -195,49 +195,50 @@ class SlackAPIFileOperator(SlackAPIOperator):
:type content: str
"""
- template_fields = ('channel', 'initial_comment', 'filename', 'filetype', 'content')
+ template_fields = ('channel', 'initial_comment', 'filetype', 'content')
ui_color = '#44BEDF'
def __init__(
self,
channel: str = '#general',
initial_comment: str = 'No message has been set!',
- filename: str = None,
+ file: str = None,
filetype: str = None,
content: str = None,
**kwargs,
) -> None:
self.method = 'files.upload'
self.channel = channel
self.initial_comment = initial_comment
- self.filename = filename
+ self.file = file
Review comment:
reverted back to filename, idea was to keep it consistent with the actual slack API expected file.params, but I understand the breakage concern.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi commented on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-895567900
> Some related slack tests are failing. Can you please rebase and fix those ?
Thanks @potiuk , Updated classic case of not digging deeper and assuming that the failure is not related.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r684179199
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -195,49 +196,50 @@ class SlackAPIFileOperator(SlackAPIOperator):
:type content: str
"""
- template_fields = ('channel', 'initial_comment', 'filename', 'filetype', 'content')
+ template_fields = ('channel', 'initial_comment', 'filetype', 'content')
ui_color = '#44BEDF'
def __init__(
self,
channel: str = '#general',
initial_comment: str = 'No message has been set!',
- filename: str = None,
+ file: str = None,
Review comment:
@mik-laj I dont think it was working before, the file parameter. I thought I fixed the file-content in an earlier PR, but the passing file was also broken.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-895010468
Some related slack tests are failing. Can you please rebase and fix those ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r689060627
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -195,49 +195,50 @@ class SlackAPIFileOperator(SlackAPIOperator):
:type content: str
"""
- template_fields = ('channel', 'initial_comment', 'filename', 'filetype', 'content')
+ template_fields = ('channel', 'initial_comment', 'filetype', 'content')
ui_color = '#44BEDF'
def __init__(
self,
channel: str = '#general',
initial_comment: str = 'No message has been set!',
- filename: str = None,
+ file: str = None,
filetype: str = None,
content: str = None,
**kwargs,
) -> None:
self.method = 'files.upload'
self.channel = channel
self.initial_comment = initial_comment
- self.filename = filename
+ self.file = file
Review comment:
I think it's better to leave that as file_name. I see no point why we should change it to `file`
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -195,49 +195,50 @@ class SlackAPIFileOperator(SlackAPIOperator):
:type content: str
"""
- template_fields = ('channel', 'initial_comment', 'filename', 'filetype', 'content')
+ template_fields = ('channel', 'initial_comment', 'filetype', 'content')
ui_color = '#44BEDF'
def __init__(
self,
channel: str = '#general',
initial_comment: str = 'No message has been set!',
- filename: str = None,
+ file: str = None,
filetype: str = None,
content: str = None,
**kwargs,
) -> None:
self.method = 'files.upload'
self.channel = channel
self.initial_comment = initial_comment
- self.filename = filename
+ self.file = file
self.filetype = filetype
self.content = content
self.file_params = {}
super().__init__(method=self.method, **kwargs)
- def construct_api_call_params(self) -> Any:
+ def execute(self, **kwargs):
+ """
+ The SlackAPIOperator calls will not fail even if the call is not unsuccessful.
+ It should not prevent a DAG from completing in success
+ """
+ slack = SlackHook(token=self.token, slack_conn_id=self.slack_conn_id)
+
+ # If file content is passed.
if self.content is not None:
self.api_params = {
'channels': self.channel,
'content': self.content,
'initial_comment': self.initial_comment,
}
- elif self.filename is not None:
+ slack.call(self.method, data=self.api_params)
+ # If file name is passed.
+ elif self.file is not None:
self.api_params = {
'channels': self.channel,
- 'filename': self.filename,
+ 'filename': self.file,
'filetype': self.filetype,
'initial_comment': self.initial_comment,
}
- self.file_params = {'file': self.filename}
-
- def execute(self, **kwargs):
- """
- The SlackAPIOperator calls will not fail even if the call is not unsuccessful.
- It should not prevent a DAG from completing in success
- """
- if not self.api_params:
- self.construct_api_call_params()
- slack = SlackHook(token=self.token, slack_conn_id=self.slack_conn_id)
- slack.call(self.method, data=self.api_params, files=self.file_params)
+ with open(self.file, "rb") as file_name:
Review comment:
actually the `file_name` here is quite wrong. The `file_name` is actually file handle, not file_name
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r683846325
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -195,49 +196,50 @@ class SlackAPIFileOperator(SlackAPIOperator):
:type content: str
"""
- template_fields = ('channel', 'initial_comment', 'filename', 'filetype', 'content')
+ template_fields = ('channel', 'initial_comment', 'filetype', 'content')
ui_color = '#44BEDF'
def __init__(
self,
channel: str = '#general',
initial_comment: str = 'No message has been set!',
- filename: str = None,
+ file: str = None,
Review comment:
It looks like a breaking change. Is there any way we can keep backward compatibility? If not, can you add a note to the changelog? For example note, see: google provider https://github.com/apache/airflow/blob/main/airflow/providers/google/CHANGELOG.rst
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r684193248
##########
File path: airflow/providers/slack/operators/slack.py
##########
@@ -195,49 +196,50 @@ class SlackAPIFileOperator(SlackAPIOperator):
:type content: str
"""
- template_fields = ('channel', 'initial_comment', 'filename', 'filetype', 'content')
+ template_fields = ('channel', 'initial_comment', 'filetype', 'content')
ui_color = '#44BEDF'
def __init__(
self,
channel: str = '#general',
initial_comment: str = 'No message has been set!',
- filename: str = None,
+ file: str = None,
Review comment:
please let me know , I can add to CHANGELOG, but as I mentioned before, for files, we need to open the file and pass the bufferedreader to Slack API, it was not done before
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#discussion_r683187682
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -187,10 +187,14 @@ def _read(self, ti, try_number, metadata=None):
response.encoding = "utf-8"
if response.status_code == 403:
- log += "*** !!!! Please make sure that all your webservers and workers have" \
- " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
- log += "*** See more at https://airflow.apache.org/docs/apache-airflow/" \
- "stable/configurations-ref.html#secret-key\n***"
+ log += (
+ "*** !!!! Please make sure that all your webservers and workers have"
+ " the same 'secret_key' configured in 'webserver' section !!!!!\n***"
+ )
+ log += (
+ "*** See more at https://airflow.apache.org/docs/apache-airflow/"
+ "stable/configurations-ref.html#secret-key\n***"
+ )
Review comment:
I broke it and just fixed it in main (bad Jarek!)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi edited a comment on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-895567900
> Some related slack tests are failing. Can you please rebase and fix those ?
Thanks @potiuk , Updated, classic case of not digging deeper and assuming that the failure is not related to the change.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-896213783
@subkanthi it's probably related because the failing tests are of Slack
```
=================================== FAILURES ===================================
_________ TestSlackAPIFileOperator.test_api_call_params_with_file_args _________
self = <tests.providers.slack.operators.test_slack.TestSlackAPIFileOperator testMethod=test_api_call_params_with_file_args>
mock_hook = <MagicMock name='SlackHook' id='139893250056768'>
@mock.patch('airflow.providers.slack.operators.slack.SlackHook')
def test_api_call_params_with_file_args(self, mock_hook):
test_slack_conn_id = 'test_slack_conn_id'
slack_api_post_operator = SlackAPIFileOperator(
task_id='slack', slack_conn_id=test_slack_conn_id, file='./test.csv', filetype='csv'
)
> slack_api_post_operator.execute()
tests/providers/slack/operators/test_slack.py:215:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <Task(SlackAPIFileOperator): slack>, kwargs = {}
slack = <MagicMock name='SlackHook()' id='139893328790136'>
def execute(self, **kwargs):
"""
The SlackAPIOperator calls will not fail even if the call is not unsuccessful.
It should not prevent a DAG from completing in success
"""
slack = SlackHook(token=self.token, slack_conn_id=self.slack_conn_id)
# If file content is passed.
if self.content is not None:
self.api_params = {
'channels': self.channel,
'content': self.content,
'initial_comment': self.initial_comment,
}
slack.call(self.method, data=self.api_params)
# If file name is passed.
elif self.file is not None:
self.api_params = {
'channels': self.channel,
'filename': self.file,
'filetype': self.filetype,
'initial_comment': self.initial_comment,
}
> with open(self.file, "rb") as file_name:
E FileNotFoundError: [Errno 2] No such file or directory: './test.csv'
airflow/providers/slack/operators/slack.py:242: FileNotFoundError
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17400: Example dag slackfile
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17400:
URL: https://github.com/apache/airflow/pull/17400#issuecomment-892378601
some docs and test failures
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org