diff --git a/README.rst b/README.rst index f8d033bec..e3b145108 100644 --- a/README.rst +++ b/README.rst @@ -220,6 +220,14 @@ in addition to its normal output. If these warnings can interfere with your scripts or you otherwise want them disabled, simply add the ``--suppress-warnings`` flag to prevent them from being emitted. +Suppressing Warnings +"""""""""""""""""""" + +Sometimes the API responds with a error that can be ignored. For example a timeout +or nginx response that can't be parsed correctly, by default the CLI will retry +calls on these errors we've identified. If you'd like to disable this behavior for +any reason use the ``--no-retry`` flag. + Shell Completion """""""""""""""" diff --git a/linodecli/__init__.py b/linodecli/__init__.py index 95055943e..62e5130aa 100755 --- a/linodecli/__init__.py +++ b/linodecli/__init__.py @@ -83,6 +83,8 @@ def main(): # pylint: disable=too-many-branches,too-many-statements cli.output_handler.columns = parsed.format cli.defaults = not parsed.no_defaults + cli.retry_count = 0 + cli.no_retry = parsed.no_retry cli.suppress_warnings = parsed.suppress_warnings if parsed.all_columns or parsed.all: diff --git a/linodecli/api_request.py b/linodecli/api_request.py index 11cfeed10..2bfc00da9 100644 --- a/linodecli/api_request.py +++ b/linodecli/api_request.py @@ -5,6 +5,7 @@ import itertools import json import sys +import time from sys import version_info from typing import Iterable, List, Optional @@ -92,6 +93,11 @@ def do_request( if ctx.debug_request: _print_response_debug_info(result) + while _check_retry(result) and not ctx.no_retry and ctx.retry_count < 3: + time.sleep(_get_retry_after(result.headers)) + ctx.retry_count += 1 + result = method(url, headers=headers, data=body, verify=API_CA_PATH) + _attempt_warn_old_version(ctx, result) if not 199 < result.status_code < 399 and not skip_error_handling: @@ -361,3 +367,24 @@ def _handle_error(ctx, response): columns=["field", "reason"], ) sys.exit(1) + + +def _check_retry(response): + """ + Check for valid retry scenario, returns true if retry is valid + """ + if response.status_code in (408, 429): + # request timed out or rate limit exceeded + return True + + return ( + response.headers + and response.status_code == 400 + and response.headers.get("Server") == "nginx" + and response.headers.get("Content-Type") == "text/html" + ) + + +def _get_retry_after(headers): + retry_str = headers.get("Retry-After", "") + return int(retry_str) if retry_str else 0 diff --git a/linodecli/arg_helpers.py b/linodecli/arg_helpers.py index 94214ea44..59fe44255 100644 --- a/linodecli/arg_helpers.py +++ b/linodecli/arg_helpers.py @@ -16,7 +16,7 @@ from linodecli import plugins from .completion import bake_completions -from .helpers import register_args_shared +from .helpers import pagination_args_shared, register_args_shared def register_args(parser): @@ -76,21 +76,6 @@ def register_args(parser): action="store_true", help="If set, does not display headers in output.", ) - parser.add_argument( - "--page", - metavar="PAGE", - default=1, - type=int, - help="For listing actions, specifies the page to request", - ) - parser.add_argument( - "--page-size", - metavar="PAGESIZE", - default=100, - type=int, - help="For listing actions, specifies the number of items per page, " - "accepts any value between 25 and 500", - ) parser.add_argument( "--all", action="store_true", @@ -126,6 +111,11 @@ def register_args(parser): default=False, help="Prevent the truncation of long values in command outputs.", ) + parser.add_argument( + "--no-retry", + action="store_true", + help="Skip retrying on common errors like timeouts.", + ) parser.add_argument( "--column-width", type=int, @@ -143,6 +133,7 @@ def register_args(parser): "--debug", action="store_true", help="Enable verbose HTTP debug output." ) + pagination_args_shared(parser) register_args_shared(parser) return parser diff --git a/linodecli/baked/request.py b/linodecli/baked/request.py index 151c4b0c2..260b88edb 100644 --- a/linodecli/baked/request.py +++ b/linodecli/baked/request.py @@ -99,7 +99,7 @@ def _parse_request_model(schema, prefix=None, list_of_objects=False): if schema.properties is not None: for k, v in schema.properties.items(): - if v.type == "object": + if v.type == "object" and not v.readOnly and v.properties: # nested objects receive a prefix and are otherwise parsed normally pref = prefix + "." + k if prefix else k args += _parse_request_model(v, prefix=pref) diff --git a/linodecli/baked/response.py b/linodecli/baked/response.py index ff18a2848..e8999089e 100644 --- a/linodecli/baked/response.py +++ b/linodecli/baked/response.py @@ -187,13 +187,14 @@ def __init__(self, response): ) else: self.attrs = _parse_response_model(response.schema) - self.rows = response.schema.extensions.get("linode-cli-rows") + self.rows = response.extensions.get("linode-cli-rows") self.nested_list = response.extensions.get("linode-cli-nested-list") def fix_json(self, json): """ Formats JSON from the API into a list of rows """ + if self.rows: return self._fix_json_rows(json) if self.nested_list: diff --git a/linodecli/helpers.py b/linodecli/helpers.py index 38cf8ae26..bb2a5288f 100644 --- a/linodecli/helpers.py +++ b/linodecli/helpers.py @@ -63,6 +63,33 @@ def filter_markdown_links(text): return result +def pagination_args_shared(parser: ArgumentParser): + """ + Add pagination related arguments to the given + ArgumentParser that may be shared across the CLI and plugins. + """ + parser.add_argument( + "--page", + metavar="PAGE", + default=1, + type=int, + help="For listing actions, specifies the page to request", + ) + parser.add_argument( + "--page-size", + metavar="PAGESIZE", + default=100, + type=int, + help="For listing actions, specifies the number of items per page, " + "accepts any value between 25 and 500", + ) + parser.add_argument( + "--all-rows", + action="store_true", + help="Output all possible rows in the results with pagination", + ) + + def register_args_shared(parser: ArgumentParser): """ Adds certain arguments to the given ArgumentParser that may be shared across @@ -87,12 +114,6 @@ def register_args_shared(parser: ArgumentParser): "This is useful for scripting the CLI's behavior.", ) - parser.add_argument( - "--all-rows", - action="store_true", - help="Output all possible rows in the results with pagination", - ) - return parser diff --git a/linodecli/plugins/obj/__init__.py b/linodecli/plugins/obj/__init__.py index 407737f9a..a316c07ef 100644 --- a/linodecli/plugins/obj/__init__.py +++ b/linodecli/plugins/obj/__init__.py @@ -14,8 +14,7 @@ from contextlib import suppress from datetime import datetime from math import ceil -from pathlib import Path -from typing import List +from typing import Iterable, List from rich import print as rprint from rich.table import Table @@ -23,7 +22,7 @@ from linodecli.cli import CLI from linodecli.configuration import _do_get_request from linodecli.configuration.helpers import _default_thing_input -from linodecli.helpers import expand_globs +from linodecli.helpers import expand_globs, pagination_args_shared from linodecli.plugins import PluginContext, inherit_plugin_args from linodecli.plugins.obj.buckets import create_bucket, delete_bucket from linodecli.plugins.obj.config import ( @@ -70,14 +69,36 @@ except ImportError: HAS_BOTO = False +TRUNCATED_MSG = ( + "Notice: Not all results were shown. If your would " + "like to get more results, you can add the '--all-row' " + "flag to the command or use the built-in pagination flags." +) + +INVALID_PAGE_MSG = "No result to show in this page." + + +def flip_to_page(iterable: Iterable, page: int = 1): + """Given a iterable object and return a specific iteration (page)""" + iterable = iter(iterable) + for _ in range(page - 1): + try: + next(iterable) + except StopIteration: + print(INVALID_PAGE_MSG) + sys.exit(2) + + return next(iterable) + def list_objects_or_buckets( get_client, args, **kwargs -): # pylint: disable=too-many-locals,unused-argument +): # pylint: disable=too-many-locals,unused-argument,too-many-branches """ Lists buckets or objects """ parser = inherit_plugin_args(ArgumentParser(PLUGIN_BASE + " ls")) + pagination_args_shared(parser) parser.add_argument( "bucket", @@ -106,16 +127,30 @@ def list_objects_or_buckets( prefix = "" data = [] + objects = [] + sub_directories = [] + pages = client.get_paginator("list_objects_v2").paginate( + Prefix=prefix, + Bucket=bucket_name, + Delimiter="/", + PaginationConfig={"PageSize": parsed.page_size}, + ) try: - response = client.list_objects_v2( - Prefix=prefix, Bucket=bucket_name, Delimiter="/" - ) + if parsed.all_rows: + results = pages + else: + page = flip_to_page(pages, parsed.page) + if page.get("IsTruncated", False): + print(TRUNCATED_MSG) + + results = [page] except client.exceptions.NoSuchBucket: print("No bucket named " + bucket_name) sys.exit(2) - objects = response.get("Contents", []) - sub_directories = response.get("CommonPrefixes", []) + for item in results: + objects.extend(item.get("Contents", [])) + sub_directories.extend(item.get("CommonPrefixes", [])) for d in sub_directories: data.append((" " * 16, "DIR", d.get("Prefix"))) @@ -329,17 +364,31 @@ def list_all_objects( """ # this is for printing help when --help is in the args parser = inherit_plugin_args(ArgumentParser(PLUGIN_BASE + " la")) + pagination_args_shared(parser) - parser.parse_args(args) + parsed = parser.parse_args(args) client = get_client() - # all buckets buckets = [b["Name"] for b in client.list_buckets().get("Buckets", [])] for b in buckets: print() - objects = client.list_objects_v2(Bucket=b).get("Contents", []) + objects = [] + pages = client.get_paginator("list_objects_v2").paginate( + Bucket=b, PaginationConfig={"PageSize": parsed.page_size} + ) + if parsed.all_rows: + results = pages + else: + page = flip_to_page(pages, parsed.page) + if page.get("IsTruncated", False): + print(TRUNCATED_MSG) + + results = [page] + + for page in results: + objects.extend(page.get("Contents", [])) for obj in objects: size = obj.get("Size", 0) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 3ea350f05..95edd2613 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -159,3 +159,7 @@ def exec_failing_test_command(args: List[str]): ) assert process.returncode == 1 return process + + +def count_lines(text: str): + return len(list(filter(len, text.split("\n")))) diff --git a/tests/integration/obj/test_obj_plugin.py b/tests/integration/obj/test_obj_plugin.py index 030d9c3ac..d8f37e572 100644 --- a/tests/integration/obj/test_obj_plugin.py +++ b/tests/integration/obj/test_obj_plugin.py @@ -1,16 +1,19 @@ import logging -import subprocess from dataclasses import dataclass -from typing import Callable, List, Optional +from typing import Callable, Optional import pytest import requests from pytest import MonkeyPatch from linodecli.configuration.auth import _do_request -from linodecli.plugins.obj import ENV_ACCESS_KEY_NAME, ENV_SECRET_KEY_NAME +from linodecli.plugins.obj import ( + ENV_ACCESS_KEY_NAME, + ENV_SECRET_KEY_NAME, + TRUNCATED_MSG, +) from tests.integration.fixture_types import GetTestFilesType, GetTestFileType -from tests.integration.helpers import BASE_URL +from tests.integration.helpers import BASE_URL, count_lines, exec_test_command REGION = "us-southeast-1" BASE_CMD = ["linode-cli", "obj", "--cluster", REGION] @@ -77,15 +80,6 @@ def patch_keys(keys: Keys, monkeypatch: MonkeyPatch): monkeypatch.setenv(ENV_SECRET_KEY_NAME, keys.secret_key) -def exec_test_command(args: List[str]): - process = subprocess.run( - args, - stdout=subprocess.PIPE, - ) - assert process.returncode == 0 - return process - - @pytest.fixture def create_bucket( name_generator: Callable, keys: Keys, monkeypatch: MonkeyPatch @@ -176,6 +170,49 @@ def test_multi_files_multi_bucket( assert "Done" in output +def test_all_rows( + create_bucket: Callable[[Optional[str]], str], + generate_test_files: GetTestFilesType, + keys: Keys, + monkeypatch: MonkeyPatch, +): + patch_keys(keys, monkeypatch) + number = 5 + bucket_name = create_bucket() + file_paths = generate_test_files(number) + + process = exec_test_command( + BASE_CMD + + ["put"] + + [str(file.resolve()) for file in file_paths] + + [bucket_name] + ) + output = process.stdout.decode() + assert "100.0%" in output + assert "Done" in output + + process = exec_test_command( + BASE_CMD + ["ls", bucket_name, "--page-size", "2", "--page", "1"] + ) + output = process.stdout.decode() + assert TRUNCATED_MSG in output + assert count_lines(output) == 3 + + process = exec_test_command( + BASE_CMD + ["ls", bucket_name, "--page-size", "999"] + ) + output = process.stdout.decode() + assert TRUNCATED_MSG not in output + assert count_lines(output) == 5 + + process = exec_test_command( + BASE_CMD + ["ls", bucket_name, "--page-size", "2", "--all-rows"] + ) + output = process.stdout.decode() + assert TRUNCATED_MSG not in output + assert count_lines(output) == 5 + + def test_modify_access_control( keys: Keys, monkeypatch: MonkeyPatch, @@ -273,7 +310,7 @@ def test_show_usage( process = exec_test_command(BASE_CMD + ["du"]) output = process.stdout.decode() - assert "40.0 MB Total" in output + assert "MB Total" in output process = exec_test_command(BASE_CMD + ["du", bucket1]) output = process.stdout.decode() diff --git a/tests/integration/stackscripts/test_stackscripts.py b/tests/integration/stackscripts/test_stackscripts.py index 29cf8109c..5990dfab8 100644 --- a/tests/integration/stackscripts/test_stackscripts.py +++ b/tests/integration/stackscripts/test_stackscripts.py @@ -45,7 +45,7 @@ def create_stackscript(): + [ "create", "--script", - "#!/bin/bash \n $EXAMPLE_SCRIPT", + '#!/bin/bash\n# \n $EXAMPLE_SCRIPT', "--image", "linode/debian9", "--label", @@ -236,6 +236,8 @@ def test_deploy_linode_from_stackscript(create_stackscript): "create", "--stackscript_id", private_stackscript, + "--stackscript_data", + '{"foo": "bar"}', "--type", linode_plan, "--image", diff --git a/tests/unit/test_api_request.py b/tests/unit/test_api_request.py index b979ed073..9172bbbbd 100644 --- a/tests/unit/test_api_request.py +++ b/tests/unit/test_api_request.py @@ -8,6 +8,7 @@ from types import SimpleNamespace from unittest.mock import Mock, patch +import pytest import requests from linodecli import api_request @@ -475,3 +476,47 @@ def json_func(): output = stderr_buf.getvalue() assert "" == output + + def test_do_request_retry(self, mock_cli, list_operation): + mock_response = Mock(status_code=408) + with patch( + "linodecli.api_request.requests.get", return_value=mock_response + ) and pytest.raises(SystemExit): + _ = api_request.do_request(mock_cli, list_operation, None) + assert mock_cli.retry_count == 3 + + def test_check_retry(self): + mock_response = Mock(status_code=200) + output = api_request._check_retry(mock_response) + assert not output + + mock_response = Mock(status_code=408) + output = api_request._check_retry(mock_response) + assert output + + mock_response = Mock(status_code=429) + output = api_request._check_retry(mock_response) + assert output + + mock_response = Mock( + status_code=400, + headers={ + "Server": "nginx", + "Content-Type": "text/html", + }, + ) + output = api_request._check_retry(mock_response) + assert output + + def test_get_retry_after(self): + headers = {"Retry-After": "10"} + output = api_request._get_retry_after(headers) + assert output == 10 + + headers = {"Retry-After": ""} + output = api_request._get_retry_after(headers) + assert output == 0 + + headers = {} + output = api_request._get_retry_after(headers) + assert output == 0 diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 0fe4c130a..eddfbc4db 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -16,10 +16,16 @@ class MockResponse: def __init__( - self, page: int, pages: int, results: int, status_code: int = 200 + self, + page: int, + pages: int, + results: int, + status_code: int = 200, + headers: dict = None, ): self.page = page self.pages = pages + self.headers = headers self.results = results self.status_code = status_code diff --git a/tests/unit/test_helpers.py b/tests/unit/test_helpers.py index cc1141c37..dc0b4c923 100644 --- a/tests/unit/test_helpers.py +++ b/tests/unit/test_helpers.py @@ -1,4 +1,10 @@ -from linodecli.helpers import filter_markdown_links +from argparse import ArgumentParser + +from linodecli.helpers import ( + filter_markdown_links, + pagination_args_shared, + register_args_shared, +) class TestHelpers: @@ -14,3 +20,24 @@ def test_markdown_links(self): ) assert filter_markdown_links(original_text) == expected_text + + def test_pagination_args_shared(self): + parser = ArgumentParser() + pagination_args_shared(parser) + + args = parser.parse_args( + ["--page", "2", "--page-size", "50", "--all-rows"] + ) + assert args.page == 2 + assert args.page_size == 50 + assert args.all_rows + + def test_register_args_shared(self): + parser = ArgumentParser() + register_args_shared(parser) + + args = parser.parse_args( + ["--as-user", "linode-user", "--suppress-warnings"] + ) + assert args.as_user == "linode-user" + assert args.suppress_warnings