fix(hub): raise exception for all status except 200 (#2225)

* fix(hub): raise exception for all status except 200

* fix(hub): raise exception for auth failures

* test(hub): refactor docker auth tests

* test(hub): refactor docker auth tests

jina/docker/hubapi/local.py

@@ -2,7 +2,7 @@
 
 import os
 import pkgutil
-from typing import Dict, Any, Optional
+from typing import Dict, Any, List, Optional
 
 from pkg_resources import parse_version
 from setuptools import find_packages
@@ -75,30 +75,40 @@ def _list_local(logger) -> Optional[Dict[str, Any]]:
     return manifests
 
 
-def _fetch_access_token(logger):
-    """ Fetch github access token from credentials file, return as a request header """
+def _fetch_access_token(logger) -> Optional[str]:
+    """
+    Fetch github access token from credentials file, return as a request header
+
+    :param logger: the logger object with which to print
+    :return: local github access token (if found)
+    """
     logger.info('fetching github access token...')
 
     if not credentials_file().is_file():
-        logger.critical(
-            f'User not logged in. please login using command: {colored("jina hub login", attrs=["bold"])}'
+        raise HubLoginRequired(
+            f'❌ User not logged in. please login using command: {colored("jina hub login", attrs=["bold"])}'
         )
-        raise HubLoginRequired
-
     try:
         with open(credentials_file(), 'r') as cf:
             cred_yml = JAML.load(cf)
             access_token = cred_yml['access_token']
             return access_token
-    except KeyError:
-        logger.error(
-            f'Invalid access file. '
+    except Exception:
+        raise HubLoginRequired(
+            f'❌ Invalid access file. '
             f'please re-login using command: {colored("jina hub login", attrs=["bold"])}'
         )
-        raise HubLoginRequired
 
 
-def _make_hub_table_with_local(images, local_images):
+def _make_hub_table_with_local(images: Dict, local_images: Dict) -> List[str]:
+    """
+    Pretty print remote and local hub images
+
+    :param images: remote images as a dict
+    :param local_images: local images as a dict
+    :return: table content to be pretty printed as a list
+    """
+
     info_table = [
         f'found {len(images)} matched hub images',
         '{:<50s}{:<25s}{:<30s}{:<25s}{:<30s}{:<50s}'.format(
@@ -140,7 +150,13 @@ def _make_hub_table_with_local(images, local_images):
     return info_table
 
 
-def _make_hub_table(images):
+def _make_hub_table(images) -> List[str]:
+    """
+    Pretty print local hub images
+
+    :param images: local images as a dict
+    :return: table content to be pretty printed as a list
+    """
     info_table = [
         f'found {len(images)} matched hub images',
         '{:<50s}{:<25s}{:<25s}{:<30s}'.format(

jina/docker/hubapi/remote.py

@@ -1,27 +1,29 @@
-"""Module wrapping interactions with the remote Jina Hub."""
-import base64
+"""Module wrapping interactions with the remote Jina Hub API"""
+from jina.logging.logger import JinaLogger
 import json
-from typing import Dict, Sequence, Any, Optional, List, Tuple
+import base64
 from urllib.error import HTTPError
 from urllib.parse import urlencode
 from urllib.request import Request, urlopen
-
 from pkg_resources import resource_stream
+from typing import Dict, Sequence, Any, Optional, List, Tuple
+
 
 from .local import (
     _fetch_access_token,
     _make_hub_table,
     _make_hub_table_with_local,
     _load_local_hub_manifest,
 )
+from ...jaml import JAML
 from ...helper import colored
 from ...importer import ImportExtensions
-from ...jaml import JAML
 from ...logging.profile import TimeContext
+from ...excepts import HubLoginRequired
 
 
 def _list(
-    logger,
+    logger: JinaLogger,
     image_name: Optional[str] = None,
     image_kind: Optional[str] = None,
     image_type: Optional[str] = None,
@@ -74,7 +76,7 @@ def _list(
         return response
 
 
-def _fetch_docker_auth(logger) -> Tuple[str, str]:
+def _fetch_docker_auth(logger: JinaLogger) -> Tuple[str, str]:
     """Use Hub api to get docker credentials.
 
     :param logger: the logger instance
@@ -84,68 +86,58 @@ def _fetch_docker_auth(logger) -> Tuple[str, str]:
         hubapi_yml = JAML.load(fp)
         hubapi_url = hubapi_yml['hubapi']['url'] + hubapi_yml['hubapi']['docker_auth']
 
-    try:
-        with ImportExtensions(
-            required=True,
-            help_text='missing "requests" dependency, please do pip install "jina[http]"',
-        ):
-            import requests
-        headers = {
-            'Accept': 'application/json',
-            'authorizationToken': _fetch_access_token(logger),
-        }
-        response = requests.get(url=f'{hubapi_url}', headers=headers)
-        if response.status_code != requests.codes.ok:
-            logger.error(
-                f'failed to fetch docker credentials. status code {response.status_code}'
-            )
-        json_response = json.loads(response.text)
-        username = base64.b64decode(json_response['docker_username']).decode('ascii')
-        password = base64.b64decode(json_response['docker_password']).decode('ascii')
-        logger.debug(f'Successfully fetched docker creds for user')
-        return username, password
-    except Exception as exp:
-        logger.error(f'got an exception while fetching docker credentials {exp!r}')
-
-
-def _register_to_mongodb(logger, summary: Optional[Dict] = None):
+    with ImportExtensions(
+        required=True,
+        help_text='Missing "requests" dependency, please do pip install "jina[http]"',
+    ):
+        import requests
+
+    headers = {
+        'Accept': 'application/json',
+        'authorizationToken': _fetch_access_token(logger),
+    }
+    response = requests.get(url=f'{hubapi_url}', headers=headers)
+    if response.status_code != requests.codes.ok:
+        raise HubLoginRequired(
+            f'❌ Failed to fetch docker credentials. status code {response.status_code}'
+        )
+    json_response = json.loads(response.text)
+    username = base64.b64decode(json_response['docker_username']).decode('ascii')
+    password = base64.b64decode(json_response['docker_password']).decode('ascii')
+    logger.debug(f'✅ Successfully fetched docker creds for user')
+    return username, password
+
+
+def _register_to_mongodb(logger: JinaLogger, summary: Optional[Dict] = None):
     """Hub API Invocation to run `hub push`.
 
     :param logger: the logger instance
     :param summary: the summary dict object
     """
+    # TODO(Deepankar): implement to jsonschema based validation for summary
     logger.info('registering image to Jina Hub database...')
 
     with resource_stream('jina', '/'.join(('resources', 'hubapi.yml'))) as fp:
         hubapi_yml = JAML.load(fp)
         hubapi_url = hubapi_yml['hubapi']['url'] + hubapi_yml['hubapi']['push']
 
-    try:
-        with ImportExtensions(
-            required=True,
-            help_text='missing "requests" dependency, please do pip install "jina[http]"',
-        ):
-            import requests
-
-            headers = {
-                'Accept': 'application/json',
-                'authorizationToken': _fetch_access_token(logger),
-            }
-            response = requests.post(
-                url=f'{hubapi_url}', headers=headers, data=json.dumps(summary)
-            )
-            if response.status_code == requests.codes.ok:
-                logger.info(response.text)
-            elif response.status_code == requests.codes.unauthorized:
-                logger.critical(
-                    f'user is unauthorized to perform push operation. '
-                    f'please login using command: {colored("jina hub login", attrs=["bold"])}'
-                )
-            elif response.status_code == requests.codes.internal_server_error:
-                logger.critical(
-                    f'got an error from the API: {response.text}. If there are any authentication issues, '
-                    f'please remember to login using command: '
-                    f'{colored("jina hub login", attrs=["bold"])}'
-                )
-    except Exception as exp:
-        logger.error(f'got an exception while invoking hubapi for push {exp!r}')
+    with ImportExtensions(
+        required=True,
+        help_text='Missing "requests" dependency, please do pip install "jina[http]"',
+    ):
+        import requests
+
+    headers = {
+        'Accept': 'application/json',
+        'authorizationToken': _fetch_access_token(logger),
+    }
+    response = requests.post(
+        url=f'{hubapi_url}', headers=headers, data=json.dumps(summary)
+    )
+    if response.status_code == requests.codes.ok:
+        logger.success(f'✅ Successfully updated the database. {response.text}')
+    else:
+        raise HubLoginRequired(
+            f'❌ Got an error from the API: {response.text.rstrip()}. '
+            f'Please login using command: {colored("jina hub login", attrs=["bold"])}'
+        )

jina/docker/hubio.py

@@ -18,10 +18,10 @@
 from .. import __version__ as jina_version
 from ..enums import BuildTestLevel
 from ..excepts import (
-    DockerLoginFailed,
     HubBuilderError,
     HubBuilderBuildError,
     HubBuilderTestError,
+    HubLoginRequired,
     ImageAlreadyExists,
 )
 from ..executors import BaseExecutor
@@ -166,7 +166,7 @@ def login(self) -> None:
                     '3. Come back to this terminal\n'
                 )
                 # allowing sometime for the user to view the message
-                time.sleep(0.5)
+                time.sleep(1)
                 webbrowser.open(verification_uri, new=2)
             except:
                 pass  # intentional pass, browser support isn't cross-platform
@@ -193,13 +193,13 @@ def login(self) -> None:
                     token = {'access_token': access_token_response['access_token']}
                     with open(credentials_file(), 'w') as cf:
                         JAML.dump(token, cf)
-                    self.logger.success(f'successfully logged in!')
+                    self.logger.success(f'✅ Successfully logged in!')
                     break
             else:
-                self.logger.error(f'max retries {login_max_retry} reached')
+                self.logger.error(f'❌ Max retries {login_max_retry} reached')
 
         except KeyError as exp:
-            self.logger.error(f'can not read the key in response: {exp}')
+            self.logger.error(f'❌ Can not read the key in response: {exp}')
 
     def list(self) -> Optional[List[Dict[str, Any]]]:
         """List all hub images given a filter specified by CLI.
@@ -279,7 +279,7 @@ def push(
 
         except Exception as e:
             self.logger.error(f'Error when trying to push image {name}: {e!r}')
-            if isinstance(e, ImageAlreadyExists):
+            if isinstance(e, (ImageAlreadyExists, HubLoginRequired)):
                 raise e
 
     def _push_docker_hub(self, name: Optional[str] = None) -> None:
@@ -367,9 +367,11 @@ def _docker_login(self) -> None:
                 password=self.args.password,
                 registry=self.args.registry,
             )
-            self.logger.debug(f'successfully logged in to docker hub')
+            self.logger.success(f'✅ Successfully logged in to docker hub')
         except APIError:
-            raise DockerLoginFailed(f'invalid credentials passed. docker login failed')
+            raise HubLoginRequired(
+                f'❌ Invalid docker credentials passed. docker login failed'
+            )
 
     def build(self) -> Dict:
         """

jina/excepts.py

@@ -124,10 +124,6 @@ class TimedOutException(Exception):
     """Errors raised for timeout operations."""
 
 
-class DockerLoginFailed(Exception):
-    """Exception to raise for docker hub login failures."""
-
-
 class ModelCheckpointNotExist(FileNotFoundError):
     """Exception to raise for executors depending on pretrained model files when they do not exist."""