Example dag slackfile #17400

subkanthi · 2021-08-03T23:12:41Z

closes: #17368

Fixed bug with SlackAPIFileOperator with uploading file.
Added example dag to demonstrate support for uploading file and file content.

…slack_upload_file

Fixed indentation. Co-authored-by: Tzu-ping Chung <[email protected]>

…slack_upload_file

…g_slackfile � Conflicts: � airflow/providers/slack/operators/slack.py

potiuk · 2021-08-04T05:39:41Z

some docs and test failures

github-actions · 2021-08-04T05:39:43Z

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.

…g_slackfile

eladkal · 2021-08-05T07:08:51Z

airflow/utils/log/file_task_handler.py

+                    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***"
+                    )


This isn't related to this PR

I broke it and just fixed it in main (bad Jarek!)

Needs rebase

mik-laj · 2021-08-05T23:22:49Z

airflow/providers/slack/operators/slack.py

    ui_color = '#44BEDF'

    def __init__(
        self,
        channel: str = '#general',
        initial_comment: str = 'No message has been set!',
-        filename: str = None,
+        file: str = None,


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

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

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

…g_slackfile

potiuk · 2021-08-09T07:28:17Z

Some related slack tests are failing. Can you please rebase and fix those ?

…g_slackfile

subkanthi · 2021-08-09T21:37:40Z

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.

eladkal · 2021-08-10T18:22:10Z

@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

subkanthi · 2021-08-10T18:46:24Z

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

…g_slackfile

potiuk

the file/file_name meaning is reversed here (and add backwards_compatibility change) - they should be swapped

airflow/providers/slack/operators/slack.py

subkanthi · 2021-08-16T12:40:18Z

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

potiuk

LGTM

mik-laj · 2021-08-16T16:31:35Z

airflow/providers/slack/operators/slack.py

-            filename="hello_world.csv",
-            filetype="csv",
-        )
+        with open("test.txt", "rb") as file:


What's changing here? file variable looks unused.

subkanthi and others added 10 commits July 27, 2021 00:26

[14168] Fixed SlackAPIFileOperator to upload file and file content.

c68567e

[14168] Fixed Lint errors

efe7047

Merge branch 'master' of github.com:subkanthi/airflow into 14880_fix_…

adb1203

…slack_upload_file

[14168] Updated documentation for Slack API File Operator

da9ed8f

[14168] Fixed indendation in SlackAPIFileOperator docs

fa542e0

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

f1c163a

Fixed indentation. Co-authored-by: Tzu-ping Chung <[email protected]>

Merge branch 'master' of github.com:subkanthi/airflow into 14880_fix_…

2744b25

…slack_upload_file

Fixed docs comment in Slack.py

df0aa9d

Fixed flake8

ca1b411

Fixed bug with SlackAPIFileOperator with uploading file.

bbd11de

boring-cyborg bot added the area:providers label Aug 3, 2021

Merge branch 'master' of github.com:subkanthi/airflow into example_da…

a68a4ad

…g_slackfile � Conflicts: � airflow/providers/slack/operators/slack.py

potiuk approved these changes Aug 4, 2021

View reviewed changes

github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Aug 4, 2021

Added example DAG link for slack

8d49057

subkanthi requested review from mik-laj and vikramkoka as code owners August 4, 2021 19:34

subkanthi added 2 commits August 4, 2021 16:02

Merge branch 'master' of github.com:subkanthi/airflow into example_da…

e3c8242

…g_slackfile

Fix doc error

eb453cd

eladkal reviewed Aug 5, 2021

View reviewed changes

Rebase from master

de4eb53

mik-laj reviewed Aug 5, 2021

View reviewed changes

subkanthi added 2 commits August 8, 2021 09:33

Merge branch 'master' of github.com:subkanthi/airflow into example_da…

194996f

…g_slackfile

Merge branch 'master' of github.com:subkanthi/airflow into example_da…

dbbc80a

…g_slackfile

subkanthi added 3 commits August 9, 2021 16:29

Merge branch 'master' of github.com:subkanthi/airflow into example_da…

1872c5a

…g_slackfile

Fixed example DAG for slack and the comments

757e2c9

Updated slack operator tests

ec3de1e

subkanthi added 3 commits August 13, 2021 16:21

Merge branch 'master' of github.com:subkanthi/airflow into example_da…

2e4207a

…g_slackfile

Fixed slack file tests

a38c7f6

Fixed tests

fa7455d

potiuk requested changes Aug 15, 2021

View reviewed changes

airflow/providers/slack/operators/slack.py Outdated Show resolved Hide resolved

airflow/providers/slack/operators/slack.py Outdated Show resolved Hide resolved

Fixed PR review comments

dea75b1

subkanthi requested a review from potiuk August 15, 2021 20:05

Fixed file variable in example_slack.py

651fd97

potiuk approved these changes Aug 16, 2021

View reviewed changes

potiuk merged commit 2935be1 into apache:main Aug 16, 2021

mik-laj reviewed Aug 16, 2021

View reviewed changes

potiuk mentioned this pull request Aug 30, 2021

Status of testing Providers that were prepared on August 30, 2021 #17922

Closed

45 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Example dag slackfile #17400

Example dag slackfile #17400

subkanthi commented Aug 3, 2021

potiuk commented Aug 4, 2021

github-actions bot commented Aug 4, 2021

eladkal Aug 5, 2021

potiuk Aug 5, 2021

potiuk Aug 5, 2021

mik-laj Aug 5, 2021

subkanthi Aug 6, 2021

subkanthi Aug 6, 2021

potiuk commented Aug 9, 2021

subkanthi commented Aug 9, 2021 •

edited

Loading

eladkal commented Aug 10, 2021

subkanthi commented Aug 10, 2021

potiuk left a comment

subkanthi commented Aug 16, 2021

potiuk left a comment

mik-laj Aug 16, 2021

Example dag slackfile #17400

Example dag slackfile #17400

Conversation

subkanthi commented Aug 3, 2021

potiuk commented Aug 4, 2021

github-actions bot commented Aug 4, 2021

eladkal Aug 5, 2021

Choose a reason for hiding this comment

potiuk Aug 5, 2021

Choose a reason for hiding this comment

potiuk Aug 5, 2021

Choose a reason for hiding this comment

mik-laj Aug 5, 2021

Choose a reason for hiding this comment

subkanthi Aug 6, 2021

Choose a reason for hiding this comment

subkanthi Aug 6, 2021

Choose a reason for hiding this comment

potiuk commented Aug 9, 2021

subkanthi commented Aug 9, 2021 • edited Loading

eladkal commented Aug 10, 2021

subkanthi commented Aug 10, 2021

potiuk left a comment

Choose a reason for hiding this comment

subkanthi commented Aug 16, 2021

potiuk left a comment

Choose a reason for hiding this comment

mik-laj Aug 16, 2021

Choose a reason for hiding this comment

subkanthi commented Aug 9, 2021 •

edited

Loading