Repository: Netflix/metaflow
Pull request: use METAFLOW_CARD_LOCALROOT for card server
Fixes https://github.com/Netflix/metaflow/issues/2120

Problem statement:
The card viewer server should load data from METAFLOW_CARD_LOCALROOT location
We can configure the location of the metadata and cards using the METAFLOW_DATASTORE_SYSROOT_LOCAL and METAFLOW_CARD_LOCALROOT.

Unfortunately, the card viewer server doesn't use these environment variables to determine where the metadata is located. Instead, we have to run the command from the same folder where the metadata is.

Consider relying on the env variables so we don't have to change folders whenever we have to run the card server.

An alternative solution is to add a parameter to control the location of the folder. Something like this:

```
python pipeline.py card server --metadata-dir /.metaflow
```

Hints from issue discussion:
thanks for the bug report! we are looking into it

Commit message: use the correct path

Patch to review:
diff --git a/metaflow/plugins/cards/card_server.py b/metaflow/plugins/cards/card_server.py
index 7b5008410ba..4b3ce0ec8d7 100644
--- a/metaflow/plugins/cards/card_server.py
+++ b/metaflow/plugins/cards/card_server.py
@@ -20,14 +20,9 @@ class ThreadingHTTPServer(ThreadingMixIn, HTTPServer):
 from .card_client import CardContainer
 from .exception import CardNotPresentException
 from .card_resolver import resolve_paths_from_task
-from metaflow.metaflow_config import DATASTORE_LOCAL_DIR
 from metaflow import namespace
-from metaflow.exception import (
-    CommandException,
-    MetaflowNotFound,
-    MetaflowNamespaceMismatch,
-)
-
+from metaflow.exception import MetaflowNotFound
+from metaflow.plugins.datastores.local_storage import LocalStorage
 
 VIEWER_PATH = os.path.join(
     os.path.dirname(os.path.abspath(__file__)), "card_viewer", "viewer.html"
@@ -50,15 +45,20 @@ class RunWatcher(Thread):
     def __init__(self, flow_name, connection: Connection):
         super().__init__()
 
-        self._watch_file = os.path.join(
-            os.getcwd(), DATASTORE_LOCAL_DIR, flow_name, "latest_run"
+        local_root = LocalStorage.datastore_root
+        if local_root is None:
+            local_root = LocalStorage.get_datastore_root_from_config(
+                lambda _: None, create_on_absent=False
+            )
+        self._watch_file = (
+            os.path.join(local_root, flow_name, "latest_run") if local_root else None
         )
         self._current_run_id = self.get_run_id()
         self.daemon = True
         self._connection = connection
 
     def get_run_id(self):
-        if not os.path.exists(self._watch_file):
+        if self._watch_file is None or not os.path.exists(self._watch_file):
             return None
         with open(self._watch_file, "r") as f:
             return f.read().strip()


Code review comments:
diff_hunk: @@ -50,15 +45,20 @@ class RunWatcher(Thread):
     def __init__(self, flow_name, connection: Connection):
         super().__init__()
 
-        self._watch_file = os.path.join(
-            os.getcwd(), DATASTORE_LOCAL_DIR, flow_name, "latest_run"
+        local_root = LocalStorage.datastore_root
+        if local_root is None:
+            local_root = LocalStorage.get_datastore_root_from_config(
+                lambda _: None, create_on_absent=False
+            )
+        self._watch_file = (
+            os.path.join(local_root, flow_name, "latest_run") if local_root else None
path: metaflow/plugins/cards/card_server.py
text: @user1:
LGTM but just one quick question : Under what circumstances is the `self._watch_file` set to `None` ie when does `local_root` become none. 

Few things we can do : 

1. We can log to the user that the `local_root` was not found (logging in the init function). 
2. On every call to the `get_run_id` function we can also check if `self._watch_file == None` . One reason for doing this is because users can start the server before they launch the first run of a flow (in which case the file will not be present and `LocalStorage.get_datastore_root_from_config` might return None) . If it's None then we re-run the `local_root` extraction and set the `self._watch_file`.  


@author:
Thanks, done all the changes..