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

add --lifecycleRule option with improved validation #182

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Support of `-` as a valid filename in `upload-file` command. In future `-` will be an alias for standard input.
* Declare official support of Python 3.12
* Cache-Control option when uploading files
* Add `--lifecycleRule` to `create-bucket` and `update-bucket` and deprecate `--lifecycleRules` argument
* Add extra dependencies for better UX, installable with `pip install b2[full]`

### Changed
* Better help text for --corsRules
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.template
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ LABEL build-date-iso8601="${build_date}"
WORKDIR ${homedir}

COPY ${tar_path}/${tar_name} .
RUN ["pip", "install", "${tar_name}"]
RUN ["pip", "install", "${tar_name}[full]"]
ENV PATH=${homedir}/.local/bin:$$PATH


Expand Down
2 changes: 2 additions & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
include requirements.txt
include requirements-full.txt
include requirements-license.txt
include LICENSE
9 changes: 8 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ Stand-alone binaries are available for Linux and Windows; this is the most strai
You can also install it in your Python environment ([virtualenv](https://pypi.org/project/virtualenv/) is recommended) from PyPI with:

```bash
pip install b2
pip install b2[full]
```

The extra dependencies improve debugging experience and, potentially, performance of `b2` CLI, but are not strictly required.
You can install the `b2` without them:

```bash
pip install b2
```

### Installing from source
Expand Down
57 changes: 57 additions & 0 deletions b2/_cli/obj_loads.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
######################################################################
#
# File: b2/_cli/obj_loads.py
#
# Copyright 2023 Backblaze Inc. All Rights Reserved.
#
# License https://www.backblaze.com/using_b2_code.html
#
######################################################################
from __future__ import annotations

import argparse
import io
import json
from typing import TypeVar

from b2sdk.v2 import get_b2sdk_doc_urls

try:
import pydantic
kkalinowski-reef marked this conversation as resolved.
Show resolved Hide resolved
from pydantic import TypeAdapter, ValidationError
except ImportError:
pydantic = None


def convert_error_to_human_readable(validation_exc: ValidationError) -> str:
buf = io.StringIO()
for error in validation_exc.errors():
loc = '.'.join(str(loc) for loc in error['loc'])
buf.write(f' In field {loc!r} input was `{error["input"]!r}`, error: {error["msg"]}\n')
return buf.getvalue()


def describe_type(type_) -> str:
urls = get_b2sdk_doc_urls(type_)
if urls:
url_links = ', '.join(f'{name} <{url}>' for name, url in urls.items())
return f'{type_.__name__} ({url_links})'
return type_.__name__


T = TypeVar('T')


def validated_loads(data: str, expected_type: type[T] | None = None) -> T:
if expected_type is not None and pydantic is not None:
ta = TypeAdapter(expected_type)
try:
val = ta.validate_json(data)
except ValidationError as e:
errors = convert_error_to_human_readable(e)
raise argparse.ArgumentTypeError(
f'Invalid value inputted, expected {describe_type(expected_type)}, got {data!r}, more detail below:\n{errors}'
) from e
else:
val = json.loads(data)
return val
54 changes: 42 additions & 12 deletions b2/console_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
FileVersion,
KeepOrDeleteMode,
LegalHold,
LifecycleRule,
NewerFileSyncMode,
ProgressReport,
ReplicationConfiguration,
Expand Down Expand Up @@ -117,6 +118,7 @@
CREATE_BUCKET_TYPES,
DEFAULT_MIN_PART_SIZE,
)
from b2._cli.obj_loads import validated_loads
from b2._cli.shell import detect_shell
from b2._utils.filesystem import points_to_fifo
from b2.arg_parser import (
Expand Down Expand Up @@ -532,6 +534,34 @@ def _setup_parser(cls, parser):
super()._setup_parser(parser) # noqa


class LifecycleRulesMixin(Described):
"""
Use `--lifecycleRule` to set lifecycle rule for the bucket.
Multiple rules can be specified by repeating the option.

`--lifecycleRules` option is deprecated and cannot be used together with --lifecycleRule.
"""

@classmethod
def _setup_parser(cls, parser):
lifecycle_group = parser.add_mutually_exclusive_group()
lifecycle_group.add_argument(
'--lifecycleRule',
action='append',
default=[],
type=functools.partial(validated_loads, expected_type=LifecycleRule),
dest='lifecycleRules',
help="Lifecycle rule in JSON format. Can be supplied multiple times.",
)
lifecycle_group.add_argument(
'--lifecycleRules',
type=functools.partial(validated_loads, expected_type=List[LifecycleRule]),
help="(deprecated; use --lifecycleRule instead) List of lifecycle rules in JSON format.",
)

super()._setup_parser(parser) # noqa


class Command(Described):
# Set to True for commands that receive sensitive information in arguments
FORBID_LOGGING_ARGUMENTS = False
Expand Down Expand Up @@ -1041,14 +1071,15 @@ def _determine_source_metadata(


@B2.register_subcommand
class CreateBucket(DefaultSseMixin, Command):
class CreateBucket(DefaultSseMixin, LifecycleRulesMixin, Command):
"""
Creates a new bucket. Prints the ID of the bucket created.

Optionally stores bucket info, CORS rules and lifecycle rules with the bucket.
These can be given as JSON on the command line.

{DEFAULTSSEMIXIN}
{LIFECYCLERULESMIXIN}

Requires capability:

Expand All @@ -1060,21 +1091,20 @@ class CreateBucket(DefaultSseMixin, Command):

@classmethod
def _setup_parser(cls, parser):
parser.add_argument('--bucketInfo', type=json.loads)
parser.add_argument('--bucketInfo', type=validated_loads)
parser.add_argument(
'--corsRules',
type=json.loads,
type=validated_loads,
help=
"If given, the bucket will have a 'custom' CORS configuration. Accepts a JSON string."
)
parser.add_argument('--lifecycleRules', type=json.loads)
parser.add_argument(
'--fileLockEnabled',
action='store_true',
help=
"If given, the bucket will have the file lock mechanism enabled. This parameter cannot be changed after bucket creation."
)
parser.add_argument('--replication', type=json.loads)
parser.add_argument('--replication', type=validated_loads)
parser.add_argument('bucketName')
parser.add_argument('bucketType', choices=CREATE_BUCKET_TYPES)

Expand Down Expand Up @@ -2512,7 +2542,7 @@ def get_synchronizer_from_args(


@B2.register_subcommand
class UpdateBucket(DefaultSseMixin, Command):
class UpdateBucket(DefaultSseMixin, LifecycleRulesMixin, Command):
"""
Updates the ``bucketType`` of an existing bucket. Prints the ID
of the bucket updated.
Expand All @@ -2521,6 +2551,7 @@ class UpdateBucket(DefaultSseMixin, Command):
These can be given as JSON on the command line.

{DEFAULTSSEMIXIN}
{LIFECYCLERULESMIXIN}

To set a default retention for files in the bucket ``--defaultRetentionMode`` and
``--defaultRetentionPeriod`` have to be specified. The latter one is of the form "X days|years".
Expand Down Expand Up @@ -2554,14 +2585,13 @@ class UpdateBucket(DefaultSseMixin, Command):

@classmethod
def _setup_parser(cls, parser):
parser.add_argument('--bucketInfo', type=json.loads)
parser.add_argument('--bucketInfo', type=validated_loads)
kkalinowski-reef marked this conversation as resolved.
Show resolved Hide resolved
parser.add_argument(
'--corsRules',
type=json.loads,
type=validated_loads,
help=
"If given, the bucket will have a 'custom' CORS configuration. Accepts a JSON string."
)
parser.add_argument('--lifecycleRules', type=json.loads)
parser.add_argument(
'--defaultRetentionMode',
choices=(
Expand All @@ -2576,7 +2606,7 @@ def _setup_parser(cls, parser):
type=parse_default_retention_period,
metavar='period',
)
parser.add_argument('--replication', type=json.loads)
parser.add_argument('--replication', type=validated_loads)
parser.add_argument(
'--fileLockEnabled',
action='store_true',
Expand Down Expand Up @@ -3517,7 +3547,7 @@ def _get_licenses_dicts(cls) -> List[Dict]:
]
)
licenses_output = piplicenses.create_output_string(args)
licenses = json.loads(licenses_output)
licenses = validated_loads(licenses_output)
return licenses

def _fetch_license_from_url(self, url: str) -> str:
Expand Down Expand Up @@ -3602,7 +3632,7 @@ def __init__(self, b2_api: Optional[B2Api], stdout, stderr):
def run_command(self, argv):
signal.signal(signal.SIGINT, keyboard_interrupt_handler)
parser = B2.get_parser()
argcomplete.autocomplete(parser)
argcomplete.autocomplete(parser, default_completer=None)
args = parser.parse_args(argv[1:])
self._setup_logging(args, argv)

Expand Down
5 changes: 3 additions & 2 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@
]
REQUIREMENTS_BUILD = ['setuptools>=20.2']
REQUIREMENTS_BUNDLE = [
'pyinstaller~=5.12',
'pyinstaller~=5.13',
'pyinstaller-hooks-contrib>=2023.6',
"patchelf-wrapper==1.2.0;platform_system=='Linux'",
"staticx~=0.13.9;platform_system=='Linux'",
]
Expand Down Expand Up @@ -258,7 +259,7 @@ def bundle(session: nox.Session):
"""Bundle the distribution."""
session.run('pip', 'install', *REQUIREMENTS_BUNDLE, **run_kwargs)
session.run('rm', '-rf', 'build', 'dist', 'b2.egg-info', external=True, **run_kwargs)
install_myself(session, ['license'])
install_myself(session, ['license', 'full'])
session.run('b2', 'license', '--dump', '--with-packages', **run_kwargs)

session.run('pyinstaller', *session.posargs, 'b2.spec', **run_kwargs)
Expand Down
1 change: 1 addition & 0 deletions requirements-full.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pydantic>=2.0.1
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
argcomplete>=2,<4
arrow>=1.0.2,<2.0.0
b2sdk>=1.22.1,<2
b2sdk>=1.23.0,<2
docutils==0.19
idna~=2.2.0; platform_system == 'Java'
importlib-metadata~=3.3; python_version < '3.8'
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def read_requirements(extra=None):
# for example:
# $ pip install -e .[dev,test]
extras_require={
'full': read_requirements('full'),

Choose a reason for hiding this comment

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

full means it's something missing from the original, while this is here only to provide a higher order validation of some inputs. How about validation or typechecks, or even just checks?

Copy link
Author

Choose a reason for hiding this comment

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

oh, but it is missing, and we only allow installing without since we acknowledge some people may run into issues with the additional dependencies.

Then there is also this - Assume we add pycurl support, then if you are a user which want the best experience possible, and you don't care if there are binary dependencies or whatnot (and why would you as the user?), do you really want to do pip install b2[pycurl,validation]` ?

It seems like a common practice to group them (https://github.com/encode/uvicorn/blob/master/pyproject.toml#L36-L44 , https://github.com/pandas-dev/pandas/blob/main/pyproject.toml#L81 ) .
We could do add extra group for just validation boost, for these users that do really care about nitpicking their dependencies, but I'm not sure how big of group will that be.

Copy link

@kkalinowski-reef kkalinowski-reef Jul 6, 2023

Choose a reason for hiding this comment

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

Oh, but in these cases all means that literally all dependencies are added, and each of them have a hefty list of optionals that can be added as well. So it should also include doc and license, and curl and everything else in this full, shouldn't it? Currently it's just adding pydantic, which is used only for validation, isn't it?

Choose a reason for hiding this comment

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

I'm not against full, or all. I'm just saying that this full is not really full.

'doc': read_requirements('doc'),
'license': read_requirements('license'),
},
Expand Down
6 changes: 3 additions & 3 deletions test/integration/test_b2_command_line.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,13 @@ def test_basic(b2_tool, bucket_name):


def test_bucket(b2_tool, bucket_name):
rules = """[{
rule = """{
"daysFromHidingToDeleting": 1,
"daysFromUploadingToHiding": null,
"fileNamePrefix": ""
}]"""
}"""
output = b2_tool.should_succeed_json(
['update-bucket', '--lifecycleRules', rules, bucket_name, 'allPublic', *get_bucketinfo()],
['update-bucket', '--lifecycleRule', rule, bucket_name, 'allPublic', *get_bucketinfo()],
)

########## // doesn't happen on production, but messes up some tests \\ ##########
Expand Down
57 changes: 56 additions & 1 deletion test/unit/test_console_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,10 @@ def _run_command(
expected_stderr = self._normalize_expected_output(expected_stderr, format_vars)
stdout, stderr = self._get_stdouterr()
console_tool = ConsoleTool(self.b2_api, stdout, stderr)
actual_status = console_tool.run_command(['b2'] + argv)
try:
actual_status = console_tool.run_command(['b2'] + argv)
except SystemExit as e:
kkalinowski-reef marked this conversation as resolved.
Show resolved Hide resolved
actual_status = e.code

actual_stdout = self._trim_trailing_spaces(stdout.getvalue())
actual_stderr = self._trim_trailing_spaces(stderr.getvalue())
Expand Down Expand Up @@ -446,6 +449,58 @@ def test_authorize_key_without_list_buckets(self):
1,
)

def test_create_bucket__with_lifecycle_rule(self):
self._authorize_account()

rule = json.dumps(
{
"daysFromHidingToDeleting": 1,
"daysFromUploadingToHiding": None,
"fileNamePrefix": ""
}
)

self._run_command(
['create-bucket', 'my-bucket', 'allPrivate', '--lifecycleRule', rule], 'bucket_0\n', '',
0
)

def test_create_bucket__with_lifecycle_rules(self):
self._authorize_account()

rules = json.dumps(
[
{
"daysFromHidingToDeleting": 1,
"daysFromUploadingToHiding": None,
"fileNamePrefix": ""
}
]
)

self._run_command(
['create-bucket', 'my-bucket', 'allPrivate', '--lifecycleRules', rules], 'bucket_0\n',
'', 0
)

def test_create_bucket__mutually_exclusive_lifecycle_rules_options(self):
self._authorize_account()

rule = json.dumps(
{
"daysFromHidingToDeleting": 1,
"daysFromUploadingToHiding": None,
"fileNamePrefix": ""
}
)

self._run_command(
[
'create-bucket', 'my-bucket', 'allPrivate', '--lifecycleRule', rule,
'--lifecycleRules', f"[{rule}]"
], '', '', 2
)

def test_create_bucket_key_and_authorize_with_it(self):
# Start with authorizing with the master key
self._authorize_account()
Expand Down
Loading