bug: accept urls in --notify-webhooks

README.md

@@ -555,12 +555,15 @@ Options:
   --auto-restart BOOLEAN          Whether automatic algorithm restarting must be enabled
   --notify-order-events BOOLEAN   Whether notifications must be sent for order events
   --notify-insights BOOLEAN       Whether notifications must be sent for emitted insights
-  --notify-emails TEXT            A comma-separated list of 'email:subject' pairs configuring email-notifications
+  --notify-emails TEXT            A comma-separated list of 'email:subject' pairs configuring email-notifications. Wrap
+                                  a subject containing ',' in double quotes, e.g. 'a@x.com:"Fills, orders"'
   --notify-webhooks TEXT          A comma-separated list of 'url:HEADER_1=VALUE_1:HEADER_2=VALUE_2:etc' pairs
-                                  configuring webhook-notifications
+                                  configuring webhook-notifications. Wrap a url (with ',' or a ':' outside its scheme
+                                  and port) or a header (with ':' or ',') in double quotes, e.g.
+                                  '"https://x.com/hook?t=1:2":"Auth=Bearer a:b"'
   --notify-sms TEXT               A comma-separated list of phone numbers configuring SMS-notifications
   --notify-telegram TEXT          A comma-separated list of 'user/group Id:token(optional)' pairs configuring telegram-
-                                  notifications
+                                  notifications. Wrap a token containing ',' in double quotes
   --live-cash-balance TEXT        A comma-separated list of currency:amount pairs of initial cash balance
   --live-holdings TEXT            A comma-separated list of symbol:symbolId:quantity:averagePrice of initial portfolio
                                   holdings

lean/commands/cloud/live/deploy.py

@@ -11,7 +11,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import List, Tuple, Optional
+from typing import Dict, List, Tuple, Optional
 from click import prompt, option, argument, confirm
 from lean.click import LeanCommand, ensure_options, CaseInsensitiveChoice
 from lean.components.api.api_client import APIClient
@@ -152,6 +152,40 @@ def _configure_notifications(logger: Logger) -> Tuple[bool, bool, List[QCNotific
     return notify_order_events, notify_insights, notify_methods
 
 
+def _split_notify_configs(notify_option: str) -> List[List[str]]:
+    """Splits a --notify-* value into its comma-separated configurations and each of those into its ':' parts.
+
+    A part wrapped in double quotes is never split, so it can contain colons and commas.
+
+    :param notify_option: the value of the --notify-* option given by the user
+    :return: the parts of each configuration, without their quotes
+    """
+    from csv import reader
+    from re import split
+
+    # A comma is a separator only when an even number of quotes follows it
+    configs = split(r',(?=(?:[^"]*"[^"]*")*[^"]*$)', notify_option)
+    return [next(reader([config], delimiter=":"), [""]) for config in configs]
+
+
+def _parse_webhook_config(parts: List[str]) -> Tuple[str, Dict[str, str]]:
+    """Splits the parts of a 'url:HEADER_1=VALUE_1:HEADER_2=VALUE_2' webhook configuration into its address and headers.
+
+    The address can contain colons itself (scheme and port), so only the trailing 'NAME=VALUE' parts are headers.
+
+    :param parts: the ':' parts of the webhook configuration given by the user
+    :return: the address and the headers of the webhook
+    """
+    from re import match
+
+    parts = list(parts)
+    headers = []
+    while len(parts) > 1 and match(r"^[\w-]+=", parts[-1]):
+        headers.insert(0, parts.pop().split("=", 1))
+
+    return ":".join(parts), dict(headers)
+
+
 def _configure_auto_restart(logger: Logger) -> bool:
     """Interactively configures whether automatic algorithm restarting must be enabled.
 
@@ -179,12 +213,15 @@ def _configure_auto_restart(logger: Logger) -> bool:
 @option("--notify-insights", type=bool, help="Whether notifications must be sent for emitted insights")
 @option("--notify-emails",
               type=str,
-              help="A comma-separated list of 'email:subject' pairs configuring email-notifications")
+              help="A comma-separated list of 'email:subject' pairs configuring email-notifications. "
+                   "Wrap a subject containing ',' in double quotes, e.g. 'a@x.com:\"Fills, orders\"'")
 @option("--notify-webhooks",
               type=str,
-              help="A comma-separated list of 'url:HEADER_1=VALUE_1:HEADER_2=VALUE_2:etc' pairs configuring webhook-notifications")
+              help="A comma-separated list of 'url:HEADER_1=VALUE_1:HEADER_2=VALUE_2:etc' pairs configuring webhook-notifications. "
+                   "Wrap a url (with ',' or a ':' outside its scheme and port) or a header (with ':' or ',') in double quotes, e.g. '\"https://x.com/hook?t=1:2\":\"Auth=Bearer a:b\"'")
 @option("--notify-sms", type=str, help="A comma-separated list of phone numbers configuring SMS-notifications")
-@option("--notify-telegram", type=str, help="A comma-separated list of 'user/group Id:token(optional)' pairs configuring telegram-notifications")
+@option("--notify-telegram", type=str, help="A comma-separated list of 'user/group Id:token(optional)' pairs configuring telegram-notifications. "
+                   "Wrap a token containing ',' in double quotes")
 @option("--live-cash-balance",
               type=str,
               default="",
@@ -254,29 +291,24 @@ def deploy(project: str,
                                                                    kwargs, logger, no_browser=no_browser)
         notify_methods = []
         if notify_emails is not None:
-            for config in notify_emails.split(","):
-                address, subject = config.split(":")
-                notify_methods.append(QCEmailNotificationMethod(address=address, subject=subject))
+            for address, *subject in _split_notify_configs(notify_emails):
+                if len(subject) == 0:
+                    raise RuntimeError(f"Invalid --notify-emails configuration '{address}', expected 'email:subject'")
+                notify_methods.append(QCEmailNotificationMethod(address=address, subject=":".join(subject)))
 
         if notify_webhooks is not None:
-            for config in notify_webhooks.split(","):
-                address, *headers = config.split(":")
-                headers = {header.split("=")[0]: header.split("=")[1] for header in headers}
+            for parts in _split_notify_configs(notify_webhooks):
+                address, headers = _parse_webhook_config(parts)
                 notify_methods.append(QCWebhookNotificationMethod(address=address, headers=headers))
 
         if notify_sms is not None:
-            for phoneNumber in notify_sms.split(","):
-                notify_methods.append(QCSMSNotificationMethod(phoneNumber=phoneNumber))
+            for parts in _split_notify_configs(notify_sms):
+                notify_methods.append(QCSMSNotificationMethod(phoneNumber=":".join(parts)))
 
         if notify_telegram is not None:
-            for config in notify_telegram.split(","):
-                id_token_pair = config.split(":", 1)    # telegram token is like "01234567:Abc132xxx..."
-                if len(id_token_pair) == 2:
-                    chat_id, token = id_token_pair
-                    token = None if not token else token
-                    notify_methods.append(QCTelegramNotificationMethod(id=chat_id, token=token))
-                else:
-                    notify_methods.append(QCTelegramNotificationMethod(id=id_token_pair[0]))
+            # telegram token is like "01234567:Abc132xxx..."
+            for chat_id, *token in _split_notify_configs(notify_telegram):
+                notify_methods.append(QCTelegramNotificationMethod(id=chat_id, token=":".join(token) or None))
 
         cash_balance_option, holdings_option, last_cash, last_holdings = get_last_portfolio_cash_holdings(api_client, brokerage_instance, cloud_project.projectId, project)
 

tests/commands/cloud/live/test_cloud_live_commands.py

@@ -11,6 +11,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from typing import List
 from unittest import mock
 import responses
 from click.testing import CliRunner
@@ -19,7 +20,7 @@
 from lean.commands import lean
 from lean.container import container
 from lean.models.api import QCEmailNotificationMethod, QCWebhookNotificationMethod, QCSMSNotificationMethod, \
-    QCTelegramNotificationMethod, QCAuth0Authorization
+    QCTelegramNotificationMethod, QCAuth0Authorization, QCNotificationMethod
 from tests.test_helpers import create_fake_lean_cli_directory, create_qc_nodes, create_api_organization
 from tests.commands.test_live import brokerage_required_options
 
@@ -231,6 +232,9 @@ def test_cloud_live_deploy_with_tradier_using_tradier_datafeed() -> None:
                                              ("webhooks", "customAddress:header1=value1"),
                                              ("webhooks", "customAddress:header1=value1:header2=value2"),
                                              ("webhooks", "customAddress1:header1=value1:header2=value2,customAddress2:header3=value3"),
+                                             ("webhooks", "https://example.com/hook"),
+                                             ("webhooks", "https://example.com:8080/hook?id=1:header1=value1"),
+                                             ("webhooks", "https://example.com/hook:Authorization=Basic dXNlcjpwYXNz=="),
                                              ("sms", "customNumber"),
                                              ("sms", "customNumber1,customNumber2,customNumber3"),
                                              ("telegram", "customId"),
@@ -241,6 +245,18 @@ def test_cloud_live_deploy_with_tradier_using_tradier_datafeed() -> None:
                                              ("telegram", "customId1:custom:token1,customId2"),
                                              ("telegram", "customId1:custom:token1,customId2:custom:token2")])
 def test_cloud_live_deploy_with_notifications(notice_method: str, configs: str) -> None:
+    expected_webhooks = {
+        "customAddress:header1=value1": ("customAddress", {"header1": "value1"}),
+        "customAddress:header1=value1:header2=value2": ("customAddress", {"header1": "value1", "header2": "value2"}),
+        "customAddress1:header1=value1:header2=value2": ("customAddress1", {"header1": "value1", "header2": "value2"}),
+        "customAddress2:header3=value3": ("customAddress2", {"header3": "value3"}),
+        "https://example.com/hook": ("https://example.com/hook", {}),
+        "https://example.com:8080/hook?id=1:header1=value1": ("https://example.com:8080/hook?id=1",
+                                                              {"header1": "value1"}),
+        "https://example.com/hook:Authorization=Basic dXNlcjpwYXNz==": ("https://example.com/hook",
+                                                                        {"Authorization": "Basic dXNlcjpwYXNz=="})
+    }
+
     create_fake_lean_cli_directory()
 
     api_client = mock.Mock()
@@ -272,12 +288,7 @@ def test_cloud_live_deploy_with_notifications(notice_method: str, configs: str)
             notification.append(QCEmailNotificationMethod(address=address, subject=subject))
 
         elif notice_method == "webhooks":
-            address, headers = config.split(":", 1)
-            headers_dict = {}
-
-            for header in headers.split(":"):
-                key, value = header.split("=")
-                headers_dict[key] = value
+            address, headers_dict = expected_webhooks[config]
 
             notification.append(QCWebhookNotificationMethod(address=address, headers=headers_dict))
 
@@ -310,6 +321,80 @@ def test_cloud_live_deploy_with_notifications(notice_method: str, configs: str)
                                                   mock.ANY)
 
 
+@pytest.mark.parametrize("notice_method,configs,expected", [
+    ("webhooks", '"https://example.com/hook?a=1:b=2":header1=value1',
+     [QCWebhookNotificationMethod(address="https://example.com/hook?a=1:b=2", headers={"header1": "value1"})]),
+    ("webhooks", '"https://user:pw=1@example.com/hook"',
+     [QCWebhookNotificationMethod(address="https://user:pw=1@example.com/hook", headers={})]),
+    ("webhooks", 'https://example.com/hook:"Authorization=Bearer a:b"',
+     [QCWebhookNotificationMethod(address="https://example.com/hook", headers={"Authorization": "Bearer a:b"})]),
+    ("webhooks", '"https://example.com/hook?ids=1,2":header1=value1,https://example.com/other:"header2=a,b"',
+     [QCWebhookNotificationMethod(address="https://example.com/hook?ids=1,2", headers={"header1": "value1"}),
+      QCWebhookNotificationMethod(address="https://example.com/other", headers={"header2": "a,b"})]),
+    ("emails", "customAddress:Alert: order filled",
+     [QCEmailNotificationMethod(address="customAddress", subject="Alert: order filled")]),
+    ("emails", 'customAddress1:"Fills, orders",customAddress2:"a:b, c"',
+     [QCEmailNotificationMethod(address="customAddress1", subject="Fills, orders"),
+      QCEmailNotificationMethod(address="customAddress2", subject="a:b, c")]),
+    ("sms", '"customNumber1",customNumber2',
+     [QCSMSNotificationMethod(phoneNumber="customNumber1"), QCSMSNotificationMethod(phoneNumber="customNumber2")]),
+    ("telegram", 'customId1:"custom:to,ken",customId2',
+     [QCTelegramNotificationMethod(id="customId1", token="custom:to,ken"), QCTelegramNotificationMethod(id="customId2")])])
+def test_cloud_live_deploy_with_quoted_notifications(notice_method: str, configs: str,
+                                                     expected: List[QCNotificationMethod]) -> None:
+    create_fake_lean_cli_directory()
+
+    api_client = mock.Mock()
+    api_client.nodes.get_all.return_value = create_qc_nodes()
+    api_client.get.return_value = {
+        "status": "stopped",
+        "stopped": "2024-07-10 19:12:20",
+        "success": True,
+        "portfolio": {"holdings": {}, "cash": {}, "success": True}}
+    container.api_client = api_client
+    container.cloud_project_manager = mock.Mock()
+    container.cloud_runner = mock.Mock()
+
+    result = CliRunner().invoke(lean, ["cloud", "live", "Python Project", "--brokerage", "Paper Trading", "--node", "live",
+                                       "--auto-restart", "yes", "--notify-order-events", "yes", "--notify-insights", "yes",
+                                       "--live-cash-balance", "USD:100", f"--notify-{notice_method}", configs,
+                                       "--data-provider-live", "QuantConnect"])
+
+    assert result.exit_code == 0
+
+    api_client.live.start.assert_called_once_with(mock.ANY,
+                                                  mock.ANY,
+                                                  "3",
+                                                  mock.ANY,
+                                                  mock.ANY,
+                                                  True,
+                                                  mock.ANY,
+                                                  True,
+                                                  True,
+                                                  expected,
+                                                  mock.ANY,
+                                                  mock.ANY)
+
+
+def test_cloud_live_deploy_aborts_when_email_notification_has_no_subject() -> None:
+    create_fake_lean_cli_directory()
+
+    api_client = mock.Mock()
+    api_client.nodes.get_all.return_value = create_qc_nodes()
+    container.api_client = api_client
+    container.cloud_project_manager = mock.Mock()
+    container.cloud_runner = mock.Mock()
+
+    result = CliRunner().invoke(lean, ["cloud", "live", "Python Project", "--brokerage", "Paper Trading", "--node", "live",
+                                       "--auto-restart", "yes", "--notify-order-events", "yes", "--notify-insights", "yes",
+                                       "--live-cash-balance", "USD:100", "--notify-emails", "customAddress",
+                                       "--data-provider-live", "QuantConnect"])
+
+    assert result.exit_code != 0
+    assert "Invalid --notify-emails configuration 'customAddress', expected 'email:subject'" in str(result.exception)
+    api_client.live.start.assert_not_called()
+
+
 @pytest.mark.parametrize("brokerage,cash", [("Paper Trading", "USD:100"),
                                             ("Paper Trading", "USD:100,EUR:200"),
                                             ("Trading Technologies", "USD:100"),