Vitor-Avila commented on code in PR #32336:
URL: https://github.com/apache/superset/pull/32336#discussion_r1982306551


##########
superset/commands/report/execute.py:
##########
@@ -139,22 +139,37 @@ def update_report_schedule_slack_v2(self) -> None:
                     slack_recipients = 
json.loads(recipient.recipient_config_json)
                     # we need to ensure that existing reports can also fetch
                     # ids from private channels
+                    channels_list = 
get_recipients_list(slack_recipients["target"])
+                    channels_list = [channel.lstrip("#") for channel in 
channels_list]
+                    channels = get_channels_with_search(
+                        search_string=channels_list,
+                        types=[
+                            SlackChannelTypes.PRIVATE,
+                            SlackChannelTypes.PUBLIC,
+                        ],
+                        exact_match=True,
+                    )
+                    if len(channels_list) != len(channels):
+                        missing_channels = set(channels_list) - {
+                            channel["name"] for channel in channels
+                        }
+                        msg = (
+                            "Could not find the following channels: "
+                            f"{', '.join(missing_channels)}"
+                        )
+                        raise UpdateFailedError(msg)

Review Comment:
   @eschutho from my tests currently (before this PR) what's happening is that 
the execution itself is listed as successful in the execution logs, but nothing 
is sent to Slack. I thought about calling `send()` with the type set to `V1` 
again, but that would cause an infinite loop as we would validate again if 
there are scopes to migrate, attempt the migration, fail again, etc. Calling it 
with `V2` would also fail as we don't have all IDs. 
   
   Considering that the Slack deprecation is next Tuesday,  I think it makes 
sense to consider any issues to migrate to V2 an error that should be fixed 
asap. Let me know your thoughts -- happy to think of other alternatives.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to