[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #8196: chore: fix version info

[GitBox <gi...@apache.org>]

mistercrunch commented on a change in pull request #8196: chore: fix version info
URL: https://github.com/apache/incubator-superset/pull/8196#discussion_r322488093
 
 

 ##########
 File path: superset/config.py
 ##########
 @@ -50,21 +50,25 @@
 # ---------------------------------------------------------
 PACKAGE_DIR = os.path.join(BASE_DIR, "static", "assets")
 VERSION_INFO_FILE = os.path.join(PACKAGE_DIR, "version_info.json")
-PACKAGE_JSON_FILE = os.path.join(PACKAGE_DIR, "package.json")
+PACKAGE_JSON_FILE = os.path.join(BASE_DIR, "assets" "package.json")
+
+
+def _try_json_readfile(filepath):
+    try:
+        with open(filepath, "r") as f:
+            return json.load(f).get("version")
+    except Exception:
+        return None
+
 
-#
 # Depending on the context in which this config is loaded, the version_info.json file
 # may or may not be available, as it is generated on install via setup.py. In the event
 # that we're actually running Superset, we will have already installed, therefore it WILL
 # exist. When unit tests are running, however, it WILL NOT exist, so we fall
 # back to reading package.json
-#
-try:
-    with open(VERSION_INFO_FILE) as version_file:
-        VERSION_STRING = json.load(version_file)["version"]
-except Exception:
-    with open(PACKAGE_JSON_FILE) as version_file:
-        VERSION_STRING = json.load(version_file)["version"]
+VERSION_STRING = _try_json_readfile(VERSION_INFO_FILE) or _try_json_readfile(
 
 Review comment:
   Yes that was the intention, `VERSION_STRING` is only used for the cli's `superset version` which isn't super important.
   
   But from this point:
   * `package.json` should always be there (under `assets/` and not `static/`), whether in the repo or in a setuptools-packaged egg / tarball, ... unless someone removes it in some way
   * `version_info.json` gets packaged when `setup.py` is invoked, and adds a SHA key, that one may or may not be there. we may want to deprecate that as it's highly contextual and hard to predict when `setup.py` may be executed, and whether a `.git/` exists at that time...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org