Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combined PR to fix issues #3, #4, #5 and PR #20 #23

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Combined PR to fix issues #3, #4, #5 and PR #20 #23

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5155996
Change: Support developer auth flow
Apr 5, 2021
5138022
Fix: do not try to authenticate when requesting help (#4)
Apr 5, 2021
4887a9a
Fix: Use urllib to combine urls (#5)
Apr 5, 2021
96fa301
Change: Format urls properly for urllib.parse.urljoin
Apr 6, 2021
2a39719
Fix: String formatting does not work like in C#
Apr 10, 2021
859c851
Fix #3: Show more meaningful errors to cli users
Apr 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions bananas_cli/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import click
import logging

from aiohttp.client_exceptions import ClientConnectorError
from .authentication import authenticate
from .helpers import task
from .session import Session
from .exceptions import Exit

log = logging.getLogger(__name__)
pass_session = click.make_pass_decorator(Session)
Expand All @@ -19,9 +21,10 @@
@click.option("--tus-url", help="BaNaNaS tus URL. (normally the same as --api-url)", metavar="URL")
@click.option("--client-id", help="Client-id to use for authentication", default="ape", show_default=True)
@click.option("--audience", help="Audience to use for authentication", default="github", show_default=False)
@click.option("--verbose", help="Enable verbose output for errors, showing tracebacks", is_flag=True)
@click.pass_context
@task
async def cli(ctx, api_url, tus_url, client_id, audience):
async def cli(ctx, api_url, tus_url, client_id, audience, verbose):
"""
A CLI tool to list, upload, and otherwise modify BaNaNaS content.

Expand All @@ -40,7 +43,7 @@ async def cli(ctx, api_url, tus_url, client_id, audience):
if not tus_url:
tus_url = api_url

session = Session(api_url, tus_url)
session = Session(api_url, tus_url, verbose)
ctx.obj = session

await session.start()
Expand All @@ -49,7 +52,14 @@ async def cli(ctx, api_url, tus_url, client_id, audience):
if "-h" in os_args or "--help" in os_args:
return

await authenticate(session, client_id, audience)
try:
await authenticate(session, client_id, audience)
except (ClientConnectorError, NameError) as e:
if verbose:
log.exception(e)
else:
log.error(e)
raise Exit


@task
Expand Down
3 changes: 3 additions & 0 deletions bananas_cli/commands/upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ def show_validation_errors(data):
@pass_session
@task
async def upload(session, version, name, description, url, license, files):
if len(files) == 0:
log.error("No files specified for upload")
return
parts = files[0].split("/")[:-1]
for filename in files:
check_parts = filename.split("/")
Expand Down
10 changes: 7 additions & 3 deletions bananas_cli/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@


class Session:
def __init__(self, api_url, tus_url):
def __init__(self, api_url, tus_url, verbose):
self.session = None
self.api_url = f"{api_url}/"
self.tus_url = f"{tus_url}/"

self._headers = {}
self.verbose = verbose

async def start(self):
self.session = aiohttp.ClientSession()
Expand Down Expand Up @@ -64,8 +65,11 @@ def tus_upload(self, upload_token, fullpath, filename):
metadata={"filename": filename, "upload-token": upload_token},
)
uploader.upload()
except TusCommunicationError:
log.exception(f"Failed to upload file '{filename}'")
except TusCommunicationError as e:
if self.verbose:
log.exception(f"Failed to upload file '{filename}'")
else:
log.error(f"Failed to upload file '{filename}': {e}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this gives better errors that the user understands sufficiently. Do you have an example of this? In my experience, the str(e) often gives something impossible to understand :D

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first iteration just said "failed to upload file {filename}", but that took away all agency from me as a command line user. I think if you are using the command line, you probably are more comfortable understanding error messages than a clicky person.

Aanyway:
If not specifying TUS url:
2021-04-12 17:21:01 ERROR Failed to upload file 'xxxx.grf': Attempt to retrieve create file url with status 404

With --verbose specified:

2021-04-12 17:22:19 ERROR    Failed to upload file 'xxxx.grf'
Traceback (most recent call last):
  File "/code/bananas-frontend-cli/bananas_cli/session.py", line 62, in tus_upload
    uploader = tus.uploader(
  File "/code/bananas-frontend-cli/.env/lib/python3.8/site-packages/tusclient/client.py", line 52, in uploader
    return Uploader(*args, **kwargs)
  File "/code/bananas-frontend-cli/.env/lib/python3.8/site-packages/tusclient/uploader.py", line 124, in __init__
    self.url = url or self.get_url()
  File "/code/bananas-frontend-cli/.env/lib/python3.8/site-packages/tusclient/uploader.py", line 216, in get_url
    return self.create_url()
  File "/code/bananas-frontend-cli/.env/lib/python3.8/site-packages/tusclient/uploader.py", line 22, in _wrapper
    return func(*args, **kwargs)
  File "/code/bananas-frontend-cli/.env/lib/python3.8/site-packages/tusclient/uploader.py", line 232, in create_url
    raise TusCommunicationError(msg, resp.status_code, resp.content)
tusclient.exceptions.TusCommunicationError: Attempt to retrieve create file url with status 404

Typing https instead of http:
2021-04-12 17:23:49 ERROR Failed to upload file 'v9point8.grf': HTTPSConnectionPool(host='172.17.0.2', port=1080): Max retries exceeded with url: /new-package/tus/ (Caused by SSLError(SSLError(1, '[SSL: WRONG_VERSION_NUMBER] wrong version number (_ssl.c:1125)'))

Wrong api url:
2021-04-12 17:28:29 ERROR Cannot connect to host 172.17.0.2:443 ssl:default [Connect call failed ('172.17.0.2', 443)]

raise Exit

def set_header(self, header, value):
Expand Down