Skip to content

Commit 921dc1c

Browse files
authored
[App] Resolve inconsistency where the flow.flows property isn't recursive leading to flow overrides (#15466)
* update * update * update * update * update * resolve attachment * update * update * update * update * update * update * update * update * update * update * update * update * update
1 parent 1c26c41 commit 921dc1c

File tree

13 files changed

+195
-160
lines changed

13 files changed

+195
-160
lines changed

src/lightning_app/CHANGELOG.md

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

2020
### Changed
2121

22-
-
22+
- Changed the `flow.flows` to be recursive wont to align the behavior with the `flow.works` ([#15466](https://github.com/Lightning-AI/lightning/pull/15466))
2323

2424
-
2525

@@ -46,6 +46,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
4646

4747
### Fixed
4848

49+
4950
- Fixed writing app name and id in connect.txt file for the command CLI ([#15443](https://github.com/Lightning-AI/lightning/pull/15443))
5051

5152
-

src/lightning_app/cli/commands/app_commands.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ def _handle_command_without_client(command: str, metadata: Dict, url: str) -> No
9494
query_parameters = "&".join(provided_params)
9595
resp = requests.post(url + f"/command/{command}?{query_parameters}")
9696
assert resp.status_code == 200, resp.json()
97+
print(resp.json())
9798

9899

99100
def _handle_command_with_client(command: str, metadata: Dict, app_name: str, app_id: Optional[str], url: str):

src/lightning_app/core/app.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ def check_error_queue(self) -> None:
284284
@property
285285
def flows(self) -> List["LightningFlow"]:
286286
"""Returns all the flows defined within this application."""
287-
return [self.root] + self.root.get_all_children()
287+
return list(self.root.flows.values())
288288

289289
@property
290290
def works(self) -> List[LightningWork]:

src/lightning_app/core/flow.py

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -207,22 +207,19 @@ def _attach_backend(flow: "LightningFlow", backend):
207207
"""Attach the backend to all flows and its children."""
208208
flow._backend = backend
209209

210-
for child_flow in flow.flows.values():
211-
LightningFlow._attach_backend(child_flow, backend)
212-
213-
for struct_name in flow._structures:
214-
structure = getattr(flow, struct_name)
215-
for flow in structure.flows:
216-
LightningFlow._attach_backend(flow, backend)
217-
for work in structure.works:
218-
backend._wrap_run_method(_LightningAppRef().get_current(), work)
219-
work._backend = backend
220-
221210
for name in flow._structures:
222211
getattr(flow, name)._backend = backend
223212

224-
for work in flow.works(recurse=False):
225-
backend._wrap_run_method(_LightningAppRef().get_current(), work)
213+
for child_flow in flow.flows.values():
214+
child_flow._backend = backend
215+
for name in child_flow._structures:
216+
getattr(child_flow, name)._backend = backend
217+
218+
app = _LightningAppRef().get_current()
219+
220+
for child_work in flow.works():
221+
child_work._backend = backend
222+
backend._wrap_run_method(app, child_work)
226223

227224
def __getattr__(self, item):
228225
if item in self.__dict__.get("_paths", {}):
@@ -274,12 +271,15 @@ def state_with_changes(self):
274271
}
275272

276273
@property
277-
def flows(self):
274+
def flows(self) -> Dict[str, "LightningFlow"]:
278275
"""Return its children LightningFlow."""
279-
flows = {el: getattr(self, el) for el in sorted(self._flows)}
276+
flows = {}
277+
for el in sorted(self._flows):
278+
flow = getattr(self, el)
279+
flows[flow.name] = flow
280+
flows.update(flow.flows)
280281
for struct_name in sorted(self._structures):
281-
for flow in getattr(self, struct_name).flows:
282-
flows[flow.name] = flow
282+
flows.update(getattr(self, struct_name).flows)
283283
return flows
284284

285285
def works(self, recurse: bool = True) -> List[LightningWork]:
@@ -297,28 +297,7 @@ def works(self, recurse: bool = True) -> List[LightningWork]:
297297

298298
def named_works(self, recurse: bool = True) -> List[Tuple[str, LightningWork]]:
299299
"""Return its :class:`~lightning_app.core.work.LightningWork` with their names."""
300-
named_works = [(el, getattr(self, el)) for el in sorted(self._works)]
301-
if not recurse:
302-
return named_works
303-
for child_name in sorted(self._flows):
304-
for w in getattr(self, child_name).works(recurse=recurse):
305-
named_works.append(w)
306-
for struct_name in sorted(self._structures):
307-
for w in getattr(self, struct_name).works:
308-
named_works.append((w.name, w))
309-
return named_works
310-
311-
def get_all_children_(self, children):
312-
sorted_children = sorted(self._flows)
313-
children.extend([getattr(self, el) for el in sorted_children])
314-
for child in sorted_children:
315-
getattr(self, child).get_all_children_(children)
316-
return children
317-
318-
def get_all_children(self):
319-
children = []
320-
self.get_all_children_(children)
321-
return children
300+
return [(w.name, w) for w in self.works(recurse=recurse)]
322301

323302
def set_state(self, provided_state: Dict, recurse: bool = True) -> None:
324303
"""Method to set the state to this LightningFlow, its children and

src/lightning_app/core/queues.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,18 @@ def __init__(
234234
self.redis = redis.Redis(host=host, port=port, password=password)
235235

236236
def put(self, item: Any) -> None:
237+
from lightning_app import LightningWork
238+
239+
is_work = isinstance(item, LightningWork)
240+
241+
# TODO: Be careful to handle with a lock if another thread needs
242+
# to access the work backend one day.
243+
# The backend isn't picklable
244+
# Raises a TypeError: cannot pickle '_thread.RLock' object
245+
if is_work:
246+
backend = item._backend
247+
item._backend = None
248+
237249
value = pickle.dumps(item)
238250
queue_len = self.length()
239251
if queue_len >= WARNING_QUEUE_SIZE:
@@ -252,6 +264,10 @@ def put(self, item: Any) -> None:
252264
"If the issue persists, please contact [email protected]"
253265
)
254266

267+
# The backend isn't pickable.
268+
if is_work:
269+
item._backend = backend
270+
255271
def get(self, timeout: int = None):
256272
"""Returns the left most element of the redis queue.
257273

src/lightning_app/structures/dict.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,17 @@ def works(self):
8686
@property
8787
def flows(self):
8888
from lightning_app import LightningFlow
89-
90-
flows = []
91-
for flow in [item for item in self.values() if isinstance(item, LightningFlow)]:
92-
flows.append(flow)
93-
for child_flow in flow.flows:
94-
flows.append(child_flow)
89+
from lightning_app.structures import Dict, List
90+
91+
flows = {}
92+
for item in self.values():
93+
if isinstance(item, LightningFlow):
94+
flows[item.name] = item
95+
for child_flow in item.flows.values():
96+
flows[child_flow.name] = child_flow
97+
if isinstance(item, (Dict, List)):
98+
for child_flow in item.flows.values():
99+
flows[child_flow.name] = child_flow
95100
return flows
96101

97102
@property

src/lightning_app/structures/list.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,17 @@ def works(self):
8989
@property
9090
def flows(self):
9191
from lightning_app import LightningFlow
92-
93-
flows = []
94-
for flow in [item for item in self if isinstance(item, LightningFlow)]:
95-
flows.append(flow)
96-
for child_flow in flow.flows:
97-
flows.append(child_flow)
92+
from lightning_app.structures import Dict, List
93+
94+
flows = {}
95+
for item in self:
96+
if isinstance(item, LightningFlow):
97+
flows[item.name] = item
98+
for child_flow in item.flows.values():
99+
flows[child_flow.name] = child_flow
100+
if isinstance(item, (Dict, List)):
101+
for child_flow in item.flows.values():
102+
flows[child_flow.name] = child_flow
98103
return flows
99104

100105
@property

src/lightning_app/utilities/app_helpers.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,10 +267,12 @@ def _set_child_name(component: "Component", child: "Component", new_name: str) -
267267

268268
# the name changed, so recursively update the names of the children of this child
269269
if isinstance(child, lightning_app.core.LightningFlow):
270-
for n, c in child.flows.items():
270+
for n in child._flows:
271+
c = getattr(child, n)
272+
_set_child_name(child, c, n)
273+
for n in child._works:
274+
c = getattr(child, n)
271275
_set_child_name(child, c, n)
272-
for n, w in child.named_works(recurse=False):
273-
_set_child_name(child, w, n)
274276
for n in child._structures:
275277
s = getattr(child, n)
276278
_set_child_name(child, s, n)

src/lightning_app/utilities/tree.py

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,6 @@ def breadth_first(root: "Component", types: Type["ComponentTuple"] = None):
1717
yield from _BreadthFirstVisitor(root, types)
1818

1919

20-
def depth_first(root: "Component", types: Type["ComponentTuple"] = None):
21-
"""Returns a generator that walks through the tree of components depth-first.
22-
23-
Arguments:
24-
root: The root component of the tree
25-
types: If provided, only the component types in this list will be visited.
26-
"""
27-
yield from _DepthFirstVisitor(root, types)
28-
29-
3020
class _BreadthFirstVisitor:
3121
def __init__(self, root: "Component", types: Type["ComponentTuple"] = None) -> None:
3222
self.queue = [root]
@@ -36,11 +26,23 @@ def __iter__(self):
3626
return self
3727

3828
def __next__(self):
29+
from lightning_app.structures import Dict
30+
3931
while self.queue:
4032
component = self.queue.pop(0)
4133

4234
if isinstance(component, lightning_app.LightningFlow):
43-
self.queue += list(component.flows.values())
35+
components = [getattr(component, el) for el in sorted(component._flows)]
36+
for struct_name in sorted(component._structures):
37+
structure = getattr(component, struct_name)
38+
if isinstance(structure, Dict):
39+
values = sorted(structure.items(), key=lambda x: x[0])
40+
else:
41+
values = sorted(((v.name, v) for v in structure), key=lambda x: x[0])
42+
for _, value in values:
43+
if isinstance(value, lightning_app.LightningFlow):
44+
components.append(value)
45+
self.queue += components
4446
self.queue += component.works(recurse=False)
4547

4648
if any(isinstance(component, t) for t in self.types):

tests/tests_app/core/lightning_app/test_configure_layout.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,9 @@ def __init__(self):
126126
root = TestContentComponent()
127127
LightningApp(root)
128128
assert root._layout == [
129-
dict(name="component0", content="root.component0"),
130-
dict(name="component1", content="root.component1"),
131-
dict(name="component2", content="root.component2"),
129+
dict(name="root.component0", content="root.component0"),
130+
dict(name="root.component1", content="root.component1"),
131+
dict(name="root.component2", content="root.component2"),
132132
]
133133

134134

0 commit comments

Comments
 (0)