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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
__pycache__/
*.pyc
.env/
12 changes: 7 additions & 5 deletions bananas_cli/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Authenticate:
session = None
token_filename = None
success = False
client_id = None

@staticmethod
@routes.get("/")
Expand All @@ -35,7 +36,7 @@ async def callback(request):
status, data = await Authenticate.session.post(
"/user/token",
json={
"client_id": "ape",
"client_id": Authenticate.client_id,
"redirect_uri": "http://localhost:3977/",
"code_verifier": Authenticate.code_verifier,
"code": code,
Expand Down Expand Up @@ -78,7 +79,7 @@ async def wait_for_code():
raise Exit


async def authenticate(session, client_id):
async def authenticate(session, client_id, audience):
config_folder = click.get_app_dir("bananas-cli")
os.makedirs(config_folder, exist_ok=True)
token_filename = config_folder + "/token"
Expand All @@ -88,20 +89,21 @@ async def authenticate(session, client_id):
bearer_token = f.read()

session.set_header("Authorization", f"Bearer {bearer_token}")
status, data = await session.get("/user")
status, data = await session.get("user")
if status == 200:
return

Authenticate.token_filename = token_filename
Authenticate.session = session
Authenticate.code_verifier = secrets.token_hex(32)
Authenticate.client_id = client_id

digest = hashlib.sha256(Authenticate.code_verifier.encode()).digest()
code_challenge = base64.urlsafe_b64encode(digest).decode().rstrip("=")

status, data = await session.get(
"/user/authorize?"
"audience=github&"
"user/authorize?"
f"audience={audience}&"
"redirect_uri=http%3A%2F%2Flocalhost%3A3977%2F&"
"response_type=code&"
f"client_id={client_id}&"
Expand Down
24 changes: 20 additions & 4 deletions bananas_cli/cli.py
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 @@ -18,16 +20,18 @@
)
@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):
async def cli(ctx, api_url, tus_url, client_id, audience, verbose):
"""
A CLI tool to list, upload, and otherwise modify BaNaNaS content.

Every option can also be set via an environment variable prefixed with
BANANAS_CLI_; for example:

BANANAS_CLI_API_URL="http://localhost:8000" python -m bananas_clie
BANANAS_CLI_API_URL="http://localhost:8000" python -m bananas_cli
"""

global session
Expand All @@ -39,11 +43,23 @@ async def cli(ctx, api_url, tus_url, client_id):
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()
await authenticate(session, client_id)

os_args = click.get_os_args()
if "-h" in os_args or "--help" in os_args:
return

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
2 changes: 1 addition & 1 deletion bananas_cli/commands/list_self.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
@pass_session
@task
async def list_self(session):
status, data = await session.get("/package/self")
status, data = await session.get("package/self")
if status != 200:
log.error(f"Server returned invalid status code {status}: {data}")
raise Exit
Expand Down
9 changes: 6 additions & 3 deletions bananas_cli/commands/upload.py
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 All @@ -41,7 +44,7 @@ async def upload(session, version, name, description, url, license, files):

starting_part = "/".join(parts) + "/"

status, data = await session.post("/new-package", json={})
status, data = await session.post("new-package", json={})
if status != 200:
log.error(f"Server returned invalid status code {status}: {data}")
raise Exit
Expand All @@ -61,7 +64,7 @@ async def upload(session, version, name, description, url, license, files):
if license:
payload["license"] = license

status, data = await session.put(f"/new-package/{upload_token}", json=payload)
status, data = await session.put(f"new-package/{upload_token}", json=payload)
if status == 400:
show_validation_errors(data)
raise Exit
Expand All @@ -74,7 +77,7 @@ async def upload(session, version, name, description, url, license, files):
log.info(f"Uploading {filename} ...")
session.tus_upload(upload_token, fullpath, filename)

status, data = await session.post(f"/new-package/{upload_token}/publish", json={})
status, data = await session.post(f"new-package/{upload_token}/publish", json={})
if status == 400:
show_validation_errors(data)
raise Exit
Expand Down
37 changes: 23 additions & 14 deletions bananas_cli/session.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import aiohttp
import logging
import urllib.parse

from tusclient.client import TusClient
from tusclient.exceptions import TusCommunicationError
Expand All @@ -12,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 = api_url
self.tus_url = tus_url
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 All @@ -27,6 +29,8 @@ async def stop(self):

async def _read_response(self, response):
if response.status in (200, 201, 400, 404):
if response.content_type == "text/html":
return 302, response.url
data = await response.json()
elif response.status in (301, 302):
data = response.headers["Location"]
Expand All @@ -36,31 +40,36 @@ async def _read_response(self, response):
return response.status, data

async def get(self, url):
response = await self.session.get(f"{self.api_url}{url}", headers=self._headers, allow_redirects=False)
full_url = urllib.parse.urljoin(self.api_url, url)
response = await self.session.get(full_url, headers=self._headers, allow_redirects=False)
return await self._read_response(response)

async def post(self, url, json):
response = await self.session.post(
f"{self.api_url}{url}", json=json, headers=self._headers, allow_redirects=False
)
full_url = urllib.parse.urljoin(self.api_url, url)
response = await self.session.post(full_url, json=json, headers=self._headers, allow_redirects=False)
return await self._read_response(response)

async def put(self, url, json):
response = await self.session.put(
f"{self.api_url}{url}", json=json, headers=self._headers, allow_redirects=False
)
full_url = urllib.parse.urljoin(self.api_url, url)
response = await self.session.put(full_url, json=json, headers=self._headers, allow_redirects=False)
return await self._read_response(response)

def tus_upload(self, upload_token, fullpath, filename):
tus = TusClient(f"{self.tus_url}/new-package/tus/")
full_url = urllib.parse.urljoin(self.tus_url, "new-package/tus/")
tus = TusClient(full_url)

try:
uploader = tus.uploader(
fullpath, chunk_size=UPLOAD_CHUNK_SIZE, metadata={"filename": filename, "upload-token": upload_token}
fullpath,
chunk_size=UPLOAD_CHUNK_SIZE,
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