Skip to content

Commit

Permalink
[cards] bug fix with error card renders (#1893)
Browse files Browse the repository at this point in the history
- Error cards didn't contain a reload token replacement because of which the UI keeps trying to poll for new cards.
- This is not the best situation because the UI will keep refreshing the card constantly until the UI catches hold of the card.
- The UI changes will obviously follow for situations where a data update might be published but a card doesn't end up getting saved because of some data store issue; This PR will fix the bug and ensures that if a error card gets created instead of an expected card then the UI still gets the right reload token.
  • Loading branch information
valayDave authored Jun 20, 2024
1 parent 4fa6389 commit a014961
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 25 deletions.
61 changes: 36 additions & 25 deletions metaflow/plugins/cards/card_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,22 @@ def wrapper(*args, **kwargs):
return wrapper


def _extract_reload_token(data, task, mf_card):
if "render_seq" not in data:
return "never"

if data["render_seq"] == "final":
# final data update should always trigger a card reload to show
# the final card, hence a different token for the final update
return "final"
elif mf_card.RELOAD_POLICY == mf_card.RELOAD_POLICY_ALWAYS:
return "render-seq-%s" % data["render_seq"]
elif mf_card.RELOAD_POLICY == mf_card.RELOAD_POLICY_NEVER:
return "never"
elif mf_card.RELOAD_POLICY == mf_card.RELOAD_POLICY_ONCHANGE:
return mf_card.reload_content_token(task, data)


def update_card(mf_card, mode, task, data, timeout_value=None):
"""
This method will be responsible for creating a card/data-update based on the `mode`.
Expand Down Expand Up @@ -432,31 +448,21 @@ def update_card(mf_card, mode, task, data, timeout_value=None):
- `timeout_stack_trace` : stack trace of the function if it timed out.
"""

def _reload_token():
if "render_seq" not in data:
return "never"

if data["render_seq"] == "final":
# final data update should always trigger a card reload to show
# the final card, hence a different token for the final update
return "final"
elif mf_card.RELOAD_POLICY == mf_card.RELOAD_POLICY_ALWAYS:
return "render-seq-%s" % data["render_seq"]
elif mf_card.RELOAD_POLICY == mf_card.RELOAD_POLICY_NEVER:
return "never"
elif mf_card.RELOAD_POLICY == mf_card.RELOAD_POLICY_ONCHANGE:
return mf_card.reload_content_token(task, data)

def _add_token_html(html):
if html is None:
return None
return html.replace(mf_card.RELOAD_POLICY_TOKEN, _reload_token())
return html.replace(
mf_card.RELOAD_POLICY_TOKEN,
_extract_reload_token(data=data, task=task, mf_card=mf_card),
)

def _add_token_json(json_msg):
if json_msg is None:
return None
return {
"reload_token": _reload_token(),
"reload_token": _extract_reload_token(
data=data, task=task, mf_card=mf_card
),
"data": json_msg,
"created_on": time.time(),
}
Expand Down Expand Up @@ -740,8 +746,16 @@ def create(
# 3. `refresh` is implemented but it raises an exception. (We do nothing. Don't store anything.)
# 4. `refresh` is implemented but it times out. (We do nothing. Don't store anything.)

def _render_error_card(stack_trace):
_card = error_card()
token = _extract_reload_token(data, task, _card)
return _card.render(
task,
stack_trace=stack_trace,
).replace(mf_card.RELOAD_POLICY_TOKEN, token)

if error_stack_trace is not None and mode != "refresh":
rendered_content = error_card().render(task, stack_trace=error_stack_trace)
rendered_content = _render_error_card(error_stack_trace)
elif (
rendered_info.is_implemented
and rendered_info.timed_out
Expand All @@ -753,18 +767,15 @@ def create(
"To increase the timeout duration for card rendering, please set the `timeout` parameter in the @card decorator. "
"\nStack Trace : \n%s"
) % (timeout, rendered_info.timeout_stack_trace)
rendered_content = error_card().render(
task,
stack_trace=timeout_stack_trace,
)
rendered_content = _render_error_card(timeout_stack_trace)
elif (
rendered_info.is_implemented
and rendered_info.data is None
and render_error_card
and mode != "refresh"
):
rendered_content = error_card().render(
task, stack_trace="No information rendered from card of type %s" % type
rendered_content = _render_error_card(
"No information rendered from card of type %s" % type
)
elif (
not rendered_info.is_implemented
Expand All @@ -775,7 +786,7 @@ def create(
"Card of type %s is a runtime time card with no `render_runtime` implemented. "
"Please implement `render_runtime` method to allow rendering this card at runtime."
) % type
rendered_content = error_card().render(task, stack_trace=message)
rendered_content = _render_error_card(message)

# todo : should we save native type for error card or error type ?
if type is not None and re.match(CARD_ID_PATTERN, type) is not None:
Expand Down
13 changes: 13 additions & 0 deletions metaflow/plugins/cards/card_modules/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,11 +542,24 @@ class ErrorCard(MetaflowCard):

type = "error"

RELOAD_POLICY = MetaflowCard.RELOAD_POLICY_ONCHANGE

def __init__(self, options={}, components=[], graph=None):
self._only_repr = True
self._graph = None if graph is None else transform_flow_graph(graph)
self._components = components

def reload_content_token(self, task, data):
"""
The reload token will change when the component array has changed in the Metaflow card.
The change in the component array is signified by the change in the component_update_ts.
"""
if task.finished:
return "final"
# `component_update_ts` will never be None. It is set to a default value when the `ComponentStore` is instantiated
# And it is updated when components added / removed / changed from the `ComponentStore`.
return "runtime-%s" % (str(data["component_update_ts"]))

def render(self, task, stack_trace=None):
RENDER_TEMPLATE = read_file(RENDER_TEMPLATE_PATH)
JS_DATA = read_file(JS_PATH)
Expand Down

0 comments on commit a014961

Please sign in to comment.