Skip to content

Commit

Permalink
Stacked @card v1.2 : New MetaflowCardComponents (#896)
Browse files Browse the repository at this point in the history
* user-facing `MetaflowCardComponent`s
- import via `from metaflow.cards import Artifact,..`

* 7 Major Changes:
- Removed `Section` component from `metaflow.cards`
- Making user-facing component inherent to `UserComponent` which in turn inherits `MetaflowCardComponent`
- Wrap all `UserComponents` inside a section after rendering everything per card
- created a `render_safely` decorator to ensure fail-safe render of `UserComponent`s
- removed code from component serializer which used internal components
- Refactored some components that return render
- Added docstrings to all components.

* JS + CSS + Cards UI Build

* Stacked @card v1.2 : Graph Related Changes to card cli (#911)

* accomodating changes from #833
- Minor tweaks to `graph.py`

* Stacked @card v1.2 : Namespace Packages with `@card` (#897)

* setup import of cards from `metaflow_extensions` using `metaflow.extension_support`
- Added import modules in `card_modules`
- Added hook in card decorators to add custom packages

* Added some debug statements for external imports.

* Stacked @card v1.2 : Test cases for Multiple `@card`s (#898)

* Multiple Cards Test Suite Mods (#27)

- Added `list_cards` to `CliCheck` and `MetadataCheck`
- Bringing #875 into the code
- Added a card that prints taskspec with random number

* Added test case for multiple cards

* Added tests card, summary :
- `current.cards['myid']` should be accessible when cards have an `id` argument in decorator
- `current.cards.append` should not work when there are no single default editable card.
- if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property.
- adding arbitrary information to `current.cards.append` should not break user code.
- Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable.
- If a single @card decorator is present with `id` then it `current.cards.append` should still work
- Access of `current.cards` with non existant id should not fail.
- `current.cards.append` should be accessible to the card with `customize=True`.
-

* fixed `DefaultEditableCardTest` test case
- Fixed comment
- Fixed the `customize=True` test case.

* ensure `test_pathspec_card` has no duplicates
- ensure entropy of rendered information is high enough to not overwrite a file.

* test case fix : `current.cards` to `current.card`

* Added Test case to support import of cards.
- Test case validates that broken card modules don't break metaflow
- test case validates that we are able to import cards from metaflow_extensions
- Test case validate that cards can be editable if they are importable.

* Added Env var to tests to avoid warnings added to cards.

* Added Test for card resume.

* Stacked @card v1.2: Card Dev Docs (#899)

Co-authored-by: Romain Cledat <[email protected]>

Co-authored-by: Brendan Gibson <[email protected]>
Co-authored-by: Brendan Gibson <[email protected]>
Co-authored-by: adam <[email protected]>
Co-authored-by: Romain Cledat <[email protected]>
  • Loading branch information
5 people authored Jan 25, 2022
1 parent 1936960 commit 77c35a6
Show file tree
Hide file tree
Showing 72 changed files with 4,572 additions and 533 deletions.
306 changes: 306 additions & 0 deletions docs/cards.md

Large diffs are not rendered by default.

18 changes: 8 additions & 10 deletions metaflow/cards.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
from metaflow.plugins.cards.card_client import get_cards
from metaflow.plugins.cards.card_modules.card import MetaflowCardComponent, MetaflowCard
from metaflow.plugins.cards.card_modules.components import (
Artifact,
Table,
Image,
Error,
Markdown,
)
from metaflow.plugins.cards.card_modules.basic import (
DefaultCard,
TitleComponent,
SubTitleComponent,
SectionComponent,
ImageComponent,
BarChartComponent,
LineChartComponent,
TableComponent,
DagComponent,
PageComponent,
ArtifactsComponent,
RENDER_TEMPLATE_PATH,
TaskToDict,
DefaultComponent,
ChartComponent,
TaskInfoComponent,
ErrorCard,
BlankCard,
)
2 changes: 2 additions & 0 deletions metaflow/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ def node_to_dict(name, node):
d["foreach_artifact"] = node.foreach_param
elif d["type"] == "split-parallel":
d["num_parallel"] = node.num_parallel
if node.matching_join:
d["matching_join"] = node.matching_join
return d

def populate_block(start_name, end_name):
Expand Down
21 changes: 18 additions & 3 deletions metaflow/plugins/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,32 @@ def get_plugin_cli():
_merge_lists(FLOW_DECORATORS, _ext_plugins["FLOW_DECORATORS"], "name")

# Cards
from .cards.card_modules.basic import DefaultCard, TaskSpecCard, ErrorCard
from .cards.card_modules.test_cards import TestErrorCard, TestTimeoutCard, TestMockCard
from .cards.card_modules.basic import DefaultCard, TaskSpecCard, ErrorCard, BlankCard
from .cards.card_modules.test_cards import (
TestErrorCard,
TestTimeoutCard,
TestMockCard,
TestPathSpecCard,
TestEditableCard,
TestEditableCard2,
TestNonEditableCard,
)
from .cards.card_modules import MF_EXTERNAL_CARDS

CARDS = [
DefaultCard,
TaskSpecCard,
ErrorCard,
BlankCard,
TestErrorCard,
TestTimeoutCard,
TestMockCard,
]
TestPathSpecCard,
TestEditableCard,
TestEditableCard2,
TestNonEditableCard,
BlankCard,
] + MF_EXTERNAL_CARDS
# Sidecars
from ..mflog.save_logs_periodically import SaveLogsPeriodicallySidecar
from metaflow.metadata.heartbeat import MetadataHeartBeat
Expand Down
16 changes: 1 addition & 15 deletions metaflow/plugins/cards/card_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,6 @@

id_func = id

# FIXME : Import the changes from Netflix/metaflow#833 for Graph
def serialize_flowgraph(flowgraph):
graph_dict = {}
for node in flowgraph:
graph_dict[node.name] = {
"type": node.type,
"box_next": node.type not in ("linear", "join"),
"box_ends": node.matching_join,
"next": node.out_funcs,
"doc": node.doc,
}
return graph_dict


def open_in_browser(card_path):
url = "file://" + os.path.abspath(card_path)
Expand Down Expand Up @@ -448,8 +435,7 @@ def create(
flowname = ctx.obj.flow.name
full_pathspec = "/".join([flowname, pathspec])

# todo : Import the changes from Netflix/metaflow#833 for Graph
graph_dict = serialize_flowgraph(ctx.obj.graph)
graph_dict, _ = ctx.obj.graph.output_steps()

# Components are rendered in a Step and added via `current.card.append` are added here.
component_arr = []
Expand Down
26 changes: 24 additions & 2 deletions metaflow/plugins/cards/card_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
)
import os
import tempfile
import uuid

_TYPE = type
_ID_FUNC = id
Expand Down Expand Up @@ -37,6 +38,7 @@ def __init__(
card_ds,
type,
path,
hash,
id=None,
html=None,
created_on=None,
Expand All @@ -51,6 +53,7 @@ def __init__(
self._card_id = id

# public attributes
self.hash = hash
self.type = type
self.from_resumed = from_resumed
self.origin_pathspec = origin_pathspec
Expand Down Expand Up @@ -86,7 +89,17 @@ def view(self):
webbrowser.open(url)

def _repr_html_(self):
return self.get()
main_html = []
container_id = uuid.uuid4()
main_html.append(
"<script type='text/javascript'>var mfContainerId = '%s';</script>"
% container_id
)
main_html.append(
"<div class='embed' data-container='%s'>%s</div>"
% (container_id, self.get())
)
return "\n".join(main_html)


class CardContainer:
Expand Down Expand Up @@ -131,6 +144,7 @@ def _get_card(self, index):
self._card_ds,
card_info.type,
path,
card_info.hash,
id=card_info.id,
html=None,
created_on=None,
Expand All @@ -144,7 +158,15 @@ def _repr_html_(self):
for idx, _ in enumerate(self._card_paths):
card = self._get_card(idx)
main_html.append(self._make_heading(card.type))
main_html.append("<div class='embed'>%s</div>" % card.get())
container_id = uuid.uuid4()
main_html.append(
"<script type='text/javascript'>var mfContainerId = '%s';</script>"
% container_id
)
main_html.append(
"<div class='embed' data-container='%s'>%s</div>"
% (container_id, card.get())
)
return "\n".join(main_html)


Expand Down
33 changes: 24 additions & 9 deletions metaflow/plugins/cards/card_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from metaflow.current import current
from metaflow.util import to_unicode
from .component_serializer import CardComponentCollector, get_card_class
from .card_modules import _get_external_card_package_paths


# from metaflow import get_metadata
import re
Expand Down Expand Up @@ -58,26 +60,39 @@ def _load_card_package(self):

card_modules_root = os.path.dirname(card_modules.__file__)

for path_tuple in self._walk(card_modules_root):
for path_tuple in self._walk(
card_modules_root, filter_extensions=[".html", ".js", ".css"]
):
file_path, arcname = path_tuple
yield (file_path, os.path.join("metaflow", "plugins", "cards", arcname))

def _walk(self, root):
external_card_pth_generator = _get_external_card_package_paths()
if external_card_pth_generator is None:
return
for module_pth, parent_arcname in external_card_pth_generator:
# `_get_card_package_paths` is a generator which yields
# path to the module and its relative arcname in the metaflow-extensions package.
for file_pth, rel_path in self._walk(module_pth, prefix_root=True):
arcname = os.path.join(parent_arcname, rel_path)
yield (file_pth, arcname)

def _walk(self, root, filter_extensions=[], prefix_root=False):
root = to_unicode(root) # handle files/folder with non ascii chars
prefixlen = len("%s/" % os.path.dirname(root))
prfx = "%s/" % (root if prefix_root else os.path.dirname(root))
prefixlen = len(prfx)
for path, dirs, files in os.walk(root):
for fname in files:
# ignoring filesnames which are hidden;
# TODO : Should we ignore hidden filenames
if fname[0] == ".":
continue

# TODO: This prevents redundant packaging of .py files for the
# default card. We should fix this logic to allow .py files to
# be included for custom cards.
if any(fname.endswith(s) for s in [".html", ".js", ".css"]):
p = os.path.join(path, fname)
yield p, p[prefixlen:]
if len(filter_extensions) > 0 and not any(
fname.endswith(s) for s in filter_extensions
):
continue
p = os.path.join(path, fname)
yield p, p[prefixlen:]

def _is_event_registered(self, evt_name):
return evt_name in self._called_once
Expand Down
111 changes: 111 additions & 0 deletions metaflow/plugins/cards/card_modules/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1,112 @@
import os
import traceback
from .card import MetaflowCard, MetaflowCardComponent
from metaflow.extension_support import get_modules, EXT_PKG, _ext_debug


def iter_namespace(ns_pkg):
# Specifying the second argument (prefix) to iter_modules makes the
# returned name an absolute name instead of a relative one. This allows
# import_module to work without having to do additional modification to
# the name.
import pkgutil

return pkgutil.iter_modules(ns_pkg.__path__, ns_pkg.__name__ + ".")


def _get_external_card_packages(with_paths=False):
"""
Safely extract all exteranl card modules
Args:
with_paths (bool, optional): setting `with_paths=True` will result in a list of tuples: `[( mf_extensions_parent_path , relative_path_to_module, module)]`. setting false will return a list of modules Defaults to False.
Returns:
`list` of `ModuleType` or `list` of `tuples` where each tuple if of the form (mf_extensions_parent_path , relative_path_to_module, ModuleType)
"""
import importlib

available_card_modules = []
for m in get_modules("plugins.cards"):
# Iterate submodules of metaflow_extensions.X.plugins.cards
# For example metaflow_extensions.X.plugins.cards.monitoring
card_packages = []
for fndx, card_mod, ispkg_c in iter_namespace(m.module):
try:
if not ispkg_c:
continue
cm = importlib.import_module(card_mod)
_ext_debug("Importing card package %s" % card_mod)
if with_paths:
card_packages.append((fndx.path, cm))
else:
card_packages.append(cm)
except Exception as e:
_ext_debug(
"External Card Module Import Exception \n\n %s \n\n %s"
% (str(e), traceback.format_exc())
)
if with_paths:
card_packages = [
(
os.path.abspath(
os.path.join(pth, "../../../../")
), # parent path to metaflow_extensions
os.path.join(
EXT_PKG,
os.path.relpath(m.__path__[0], os.path.join(pth, "../../../")),
), # construct relative path to parent.
m,
)
for pth, m in card_packages
]
available_card_modules.extend(card_packages)
return available_card_modules


def _load_external_cards():
# Load external card packages
card_packages = _get_external_card_packages()
if not card_packages:
return []
external_cards = {}
card_arr = []
# Load cards from all external packages.
for package in card_packages:
try:
cards = package.CARDS
# Ensure that types match.
if not type(cards) == list:
continue
except AttributeError:
continue
else:
for c in cards:
if not isinstance(c, type) or not issubclass(c, MetaflowCard):
# every card should only be inheriting a MetaflowCard
continue
if not getattr(c, "type", None):
# todo Warn user of non existant `type` in MetaflowCard
continue
if c.type in external_cards:
# todo Warn user of duplicate card
continue
# external_cards[c.type] = c
card_arr.append(c)
return card_arr


def _get_external_card_package_paths():
pkg_iter = _get_external_card_packages(with_paths=True)
if pkg_iter is None:
return None
for (
mf_extension_parent_path,
relative_path_to_module,
_,
) in pkg_iter:
module_pth = os.path.join(mf_extension_parent_path, relative_path_to_module)
arcname = relative_path_to_module
yield module_pth, arcname


MF_EXTERNAL_CARDS = _load_external_cards()
9 changes: 7 additions & 2 deletions metaflow/plugins/cards/card_modules/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@
</head>

<body>
<div id="app"></div>
<div class="card_app"></div>
<script>
window.__DATA__ = {{{task_data}}}
var mfCardDataId = "{{{card_data_id}}}";

if (!window.__MF_DATA__) {
window.__MF_DATA__ = {};
}
window.__MF_DATA__["{{{card_data_id}}}"] = {{{task_data}}}
</script>
<script>
{{{javascript}}}
Expand Down
Loading

0 comments on commit 77c35a6

Please sign in to comment.