Skip to content

Commit 7c329dd

Browse files
awaelchliBordaSherin Thomas
authored
Warn when http URLs are configured (#14233)
* add a warning * add test * add test * add changelog * remove todo * clarify http won't work in cloud * Apply suggestions from code review Co-authored-by: Sherin Thomas <sherin@grid.ai> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Sherin Thomas <sherin@grid.ai>
1 parent 60933c0 commit 7c329dd

File tree

5 files changed

+56
-4
lines changed

5 files changed

+56
-4
lines changed

src/lightning_app/CHANGELOG.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,17 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
2121

2222

2323
- Adds `LightningTrainingComponent`. `LightningTrainingComponent` orchestrates multi-node training in the cloud ([#13830](https://github.com/Lightning-AI/lightning/pull/13830))
24-
- Add support for printing application logs using CLI `lightning show logs <app_name> [components]` ([#13634](https://github.com/Lightning-AI/lightning/pull/13634))
2524

2625

26+
- Add support for printing application logs using CLI `lightning show logs <app_name> [components]` ([#13634](https://github.com/Lightning-AI/lightning/pull/13634))
27+
2728

2829
- Add support for `Lightning API` through the `configure_api` hook on the Lightning Flow and the `Post`, `Get`, `Delete`, `Put` HttpMethods ([#13945](https://github.com/Lightning-AI/lightning/pull/13945))
30+
31+
32+
- Added a warning when `configure_layout` returns URLs configured with http instead of https ([#14233](https://github.com/Lightning-AI/lightning/pull/14233))
33+
34+
2935
### Changed
3036

3137
- Default values and parameter names for Lightning AI BYOC cluster management ([#14132](https://github.com/Lightning-AI/lightning/pull/14132))

src/lightning_app/utilities/cloud.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import os
12
import warnings
23

34
from lightning_cloud.openapi import V1Membership
@@ -34,3 +35,8 @@ def _get_project(client: LightningClient, project_id: str = LIGHTNING_CLOUD_PROJ
3435

3536
def _sigterm_flow_handler(*_, app: "lightning_app.LightningApp"):
3637
app.stage = AppStage.STOPPING
38+
39+
40+
def is_running_in_cloud() -> bool:
41+
"""Returns True if the Lightning App is running in the cloud."""
42+
return "LIGHTNING_APP_STATE_URL" in os.environ

src/lightning_app/utilities/layout.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import inspect
2+
import warnings
23
from typing import Dict, List, Union
34

45
import lightning_app
56
from lightning_app.frontend.frontend import Frontend
7+
from lightning_app.utilities.cloud import is_running_in_cloud
68

79

810
def _add_comment_to_literal_code(method, contains, comment):
@@ -79,11 +81,17 @@ def _collect_content_layout(layout: List[Dict], flow: "lightning_app.LightningFl
7981
f" For the value, choose either a reference to a child flow or a URla."
8082
)
8183
if isinstance(entry["content"], str): # assume this is a URL
82-
# The URL isn't fully defined yet. Looks something like ``self.work.url + /something``.
83-
if entry["content"].startswith("/"):
84+
url = entry["content"]
85+
if url.startswith("/"):
86+
# The URL isn't fully defined yet. Looks something like ``self.work.url + /something``.
8487
entry["target"] = ""
8588
else:
86-
entry["target"] = entry["content"]
89+
entry["target"] = url
90+
if url.startswith("http://") and is_running_in_cloud():
91+
warnings.warn(
92+
f"You configured an http link {url[:32]}... but it won't be accessible in the cloud."
93+
f" Consider replacing 'http' with 'https' in the link above."
94+
)
8795
elif isinstance(entry["content"], lightning_app.LightningFlow):
8896
entry["content"] = entry["content"].name
8997
elif isinstance(entry["content"], lightning_app.LightningWork):

tests/tests_app/core/lightning_app/test_configure_layout.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,3 +218,21 @@ def test_dynamic_content_layout_update():
218218
app = LightningApp(flow)
219219
MultiProcessRuntime(app).dispatch()
220220
assert flow.configure_layout_called == 5
221+
222+
223+
@mock.patch("lightning_app.utilities.layout.is_running_in_cloud", return_value=True)
224+
def test_http_url_warning(*_):
225+
class Root(EmptyFlow):
226+
def configure_layout(self):
227+
return [
228+
dict(name="warning expected", content="http://github.com/very/long/link/to/display"),
229+
dict(name="no warning expected", content="https://github.com"),
230+
]
231+
232+
root = Root()
233+
234+
with pytest.warns(
235+
UserWarning,
236+
match=escape("You configured an http link http://github.com/very/long/link... but it won't be accessible"),
237+
):
238+
LightningApp(root)
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import os
2+
from unittest import mock
3+
4+
from lightning_app.utilities.cloud import is_running_in_cloud
5+
6+
7+
@mock.patch.dict(os.environ, clear=True)
8+
def test_is_running_locally():
9+
assert not is_running_in_cloud()
10+
11+
12+
@mock.patch.dict(os.environ, {"LIGHTNING_APP_STATE_URL": "127.0.0.1"})
13+
def test_is_running_cloud():
14+
assert is_running_in_cloud()

0 commit comments

Comments
 (0)