[GitHub] [kibble] Humbedooh commented on a change in pull request #48: Refactor setup.py

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

Humbedooh commented on a change in pull request #48:
URL: https://github.com/apache/kibble/pull/48#discussion_r502895222



##########
File path: setup/setup.py
##########
@@ -14,143 +14,93 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-KIBBLE_VERSION = '0.1.0' # ABI/API compat demarcation.
-KIBBLE_DB_VERSION = 2 # Second database revision
-
 import sys
-
-if sys.version_info <= (3, 3):
-    print("This script requires Python 3.4 or higher")
-    sys.exit(-1)
-
 import os
-import getpass
-import subprocess
 import argparse
-import shutil
+import logging
 import yaml
 import bcrypt
 import json
 
-mappings = json.load(open("mappings.json"))
-myyaml = yaml.load(open("kibble.yaml.sample"))
+KIBBLE_VERSION = '0.1.0'  # ABI/API compat demarcation.
+KIBBLE_DB_VERSION = 2  # Second database revision
+
+if sys.version_info <= (3, 3):
+    print("This script requires Python 3.4 or higher")
+    sys.exit(-1)
 
-dopip = False
+# Check if Elasticsearch is installed, if not
+# prompt user with command to run
 try:
     from elasticsearch import Elasticsearch
-    from elasticsearch import VERSION as ES_VERSION
-    ES_MAJOR = ES_VERSION[0]
-except:
-    dopip = True
-    
-if dopip and (getpass.getuser() != "root"):
-    print("It looks like you need to install some python modules first")
-    print("Either run this as root to do so, or run: ")
-    print("pip3 install elasticsearch certifi bcrypt")
+except ImportError:
+    print(
+        "It looks like you need to install some python modules first"
+        "Either run this as root to do so, or run: \n"
+        "pip3 install elasticsearch certifi bcrypt"
+    )
     sys.exit(-1)
+    
 
-elif dopip:
-    print("Before we get started, we need to install some modules")
-    print("Hang on!")
-    try:
-        subprocess.check_call(('pip3','install','elasticsearch', 'certifi', 'bcrypt'))
-        from elasticsearch import Elasticsearch
-    except:
-        print("Oh dear, looks like this failed :(")
-        print("Please install elasticsearch and certifi before you try again:")
-        print("pip install elasticsearch certifi")
-        sys.exit(-1)

Review comment:
       Telling users to utilize requirements.txt would probably be the best bet here. That way you can also use pipenv easily.




----------------------------------------------------------------
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