Skip to content

Commit

Permalink
Merge branch 'master' into put-object-options
Browse files Browse the repository at this point in the history
  • Loading branch information
drernie committed Dec 18, 2024
2 parents 9180b9b + e7743a7 commit 2d21857
Show file tree
Hide file tree
Showing 238 changed files with 9,903 additions and 3,942 deletions.
5 changes: 0 additions & 5 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ updates:
schedule:
interval: "daily"

- package-ecosystem: "docker"
directory: "/lambdas/molecule"
schedule:
interval: "daily"

- package-ecosystem: "docker"
directory: "/catalog"
schedule:
Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/deploy-lambdas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ jobs:
docker run --rm \
--entrypoint /build_zip.sh \
-v "$PWD/lambdas/${{ matrix.path }}":/lambda/function:z \
-v "$PWD/lambdas/shared":/lambda/shared:z \
-v "$PWD/out.zip":/out.zip:z \
-v "$PWD/lambdas/scripts/build_zip.sh":/build_zip.sh:z \
"$BUILDER_IMAGE"
Expand All @@ -65,7 +64,6 @@ jobs:
strategy:
matrix:
path:
- molecule
- thumbnail
runs-on: ubuntu-latest
# These permissions are needed to interact with GitHub's OIDC Token endpoint.
Expand All @@ -76,7 +74,7 @@ jobs:
- uses: actions/checkout@v4
- name: Build Docker image
working-directory: ./lambdas/${{ matrix.path }}
run: docker buildx build -t "quiltdata/lambdas/${{ matrix.path }}:${{ github.sha }}" -f Dockerfile ..
run: docker buildx build -t "quiltdata/lambdas/${{ matrix.path }}:${{ github.sha }}" -f Dockerfile .
- name: Configure AWS credentials from Prod account
uses: aws-actions/configure-aws-credentials@v4
with:
Expand Down
25 changes: 8 additions & 17 deletions .github/workflows/py-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.10'
python-version: '3.11'
- name: Install dependencies
run: |
python -m pip install --upgrade pip setuptools
python -m pip install 'pylint==2.10.2' 'pycodestyle>=2.6.1'
python -m pip install 'pylint==3.2.7' 'pycodestyle>=2.6.1'
- name: Run pylint
run: |
pylint $(find -name '*.py' -not -path './venv/*')
pylint .
- name: Run pycodestyle
run: |
pycodestyle $(find -name '*.py' -not -path './venv/*')
Expand All @@ -31,7 +31,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.10'
python-version: '3.11'
- name: Install dependencies
run: |
python -m pip install --upgrade pip setuptools
Expand Down Expand Up @@ -70,7 +70,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.10'
python-version: '3.11'
- name: install poetry
run: python -m pip install poetry
- name: install deps
Expand Down Expand Up @@ -157,7 +157,6 @@ jobs:
path:
- access_counts
- indexer
- molecule
- pkgevents
- pkgpush
- preview
Expand Down Expand Up @@ -188,23 +187,15 @@ jobs:
python -m pip install wheel 'numpy<2'
fi
# XXX: something fishy is going on with this "if [ ] X then Y" syntax
if [ ${{ matrix.path }} == "shared" ]
python -m pip install -e lambdas/shared[tests]
then
python -m pip install -e lambdas/shared
python -m pip install -e lambdas/${{ matrix.path }}
fi
python -m pip install -r lambdas/${{ matrix.path }}/test-requirements.txt
# Try to simulate the lambda .zip file:
# - Use --no-deps to ensure that second-order dependencies are included in the requirements file
# - Remove "tests" directories
# - Run "strip" on shared libraries
python -m pip install -t deps --no-deps -r lambdas/${{ matrix.path }}/requirements.txt
python -m pip install -t deps --no-deps -r lambdas/${{ matrix.path }}/requirements.txt lambdas/${{ matrix.path }}
find deps -name tests -type d -exec rm -r \{} \+
find deps \( -name '*.so.*' -o -name '*.so' \) -type f -exec strip \{} \+
python -m pip install -r lambdas/${{ matrix.path }}/test-requirements.txt
- name: Pytest
run: |
pytest --cov=lambdas lambdas/${{ matrix.path }}
Expand Down
2 changes: 1 addition & 1 deletion api/python/quilt3/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
6.1.0
6.1.1
2 changes: 1 addition & 1 deletion api/python/quilt3/data_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def find_correct_client(self, api_type, bucket, param_dict):
S3Api.LIST_OBJECTS_V2: check_list_objects_v2_works_for_client,
S3Api.LIST_OBJECT_VERSIONS: check_list_object_versions_works_for_client
}
assert api_type in check_fn_mapper.keys(), f"Only certain APIs are supported with unsigned_client. The " \
assert api_type in check_fn_mapper, f"Only certain APIs are supported with unsigned_client. The " \
f"API '{api_type}' is not current supported. You may want to use S3ClientProvider.standard_client " \
f"instead "
check_fn = check_fn_mapper[api_type]
Expand Down
30 changes: 16 additions & 14 deletions api/python/quilt3/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ def install(cls, name, registry=None, top_hash=None, dest=None, dest_registry=No

if subpkg_key is not None:
if subpkg_key not in pkg:
raise QuiltException(f"Package {name} doesn't contain {subpkg_key!r}.")
raise QuiltException(f"Package {name!r} doesn't contain {subpkg_key!r}.")
entry = pkg[subpkg_key]
entries = entry.walk() if isinstance(entry, Package) else ((subpkg_key.split('/')[-1], entry),)
else:
Expand Down Expand Up @@ -833,7 +833,7 @@ def _load(cls, readable_file):
subpkg.set_meta(obj['meta'])
continue
if key in subpkg._children:
raise PackageException("Duplicate logical key while loading package")
raise PackageException(f"Duplicate logical key {key!r} while loading package entry: {obj!r}")
subpkg._children[key] = PackageEntry(
PhysicalKey.from_url(obj['physical_keys'][0]),
obj['size'],
Expand Down Expand Up @@ -871,7 +871,7 @@ def set_dir(self, lkey, path=None, meta=None, update_policy="incoming", unversio
ValueError: When `update_policy` is invalid.
"""
if update_policy not in PACKAGE_UPDATE_POLICY:
raise ValueError(f"Update policy should be one of {PACKAGE_UPDATE_POLICY}, not {update_policy!r}")
raise ValueError(f"Update policy should be one of {PACKAGE_UPDATE_POLICY!r}, not {update_policy!r}")

lkey = lkey.strip("/")

Expand All @@ -892,7 +892,7 @@ def set_dir(self, lkey, path=None, meta=None, update_policy="incoming", unversio
if src.is_local():
src_path = pathlib.Path(src.path)
if not src_path.is_dir():
raise PackageException("The specified directory doesn't exist")
raise PackageException(f"The specified directory {src_path!r} doesn't exist")

files = src_path.rglob('*')
ignore = src_path / '.quiltignore'
Expand Down Expand Up @@ -953,7 +953,7 @@ def get(self, logical_key):
"""
obj = self[logical_key]
if not isinstance(obj, PackageEntry):
raise ValueError("Key does not point to a PackageEntry")
raise ValueError(f"Key {logical_key!r} does not point to a PackageEntry")
return obj.get()

def readme(self):
Expand Down Expand Up @@ -1192,8 +1192,7 @@ def _set(
):
if not logical_key or logical_key.endswith('/'):
raise QuiltException(
f"Invalid logical key {logical_key!r}. "
f"A package entry logical key cannot be a directory."
f"A package entry logical key {logical_key!r} must be a file."
)

validate_key(logical_key)
Expand Down Expand Up @@ -1240,7 +1239,7 @@ def _set(
if len(format_handlers) == 0:
error_message = f'Quilt does not know how to serialize a {type(entry)}'
if ext is not None:
error_message += f' as a {ext} file.'
error_message += f' as a {ext!r} file.'
error_message += '. If you think this should be supported, please open an issue or PR at ' \
'https://github.com/quiltdata/quilt'
raise QuiltException(error_message)
Expand Down Expand Up @@ -1273,7 +1272,7 @@ def _set(

pkg = self._ensure_subpackage(path[:-1], ensure_no_entry=True)
if path[-1] in pkg and isinstance(pkg[path[-1]], Package):
raise QuiltException("Cannot overwrite directory with PackageEntry")
raise QuiltException(f"Cannot overwrite directory {path[-1]!r} with PackageEntry")
pkg._children[path[-1]] = entry

return self
Expand All @@ -1294,7 +1293,10 @@ def _ensure_subpackage(self, path, ensure_no_entry=False):
for key_fragment in path:
if ensure_no_entry and key_fragment in pkg \
and isinstance(pkg[key_fragment], PackageEntry):
raise QuiltException("Already a PackageEntry along the path.")
raise QuiltException(
f"Already a PackageEntry for {key_fragment!r} "
f"along the path {path!r}: {pkg[key_fragment].physical_key!r}",
)
pkg = pkg._children.setdefault(key_fragment, Package())
return pkg

Expand Down Expand Up @@ -1344,7 +1346,7 @@ def _get_top_hash_parts(cls, meta, entries):
for logical_key, entry in entries:
if entry.hash is None or entry.size is None:
raise QuiltException(
"PackageEntry missing hash and/or size: %s" % entry.physical_key
"PackageEntry missing hash and/or size: %r" % entry.physical_key
)
yield {
'hash': entry.hash,
Expand Down Expand Up @@ -1456,7 +1458,7 @@ def dest_fn(*args, **kwargs):
raise TypeError(f'{dest!r} returned {url!r}, but str is expected')
pk = PhysicalKey.from_url(url)
if pk.is_local():
raise util.URLParseError("Unexpected scheme: 'file'")
raise util.URLParseError(f"Unexpected scheme: 'file' for {pk!r}")
if pk.version_id:
raise ValueError(f'{dest!r} returned {url!r}, but URI must not include versionId')
return pk
Expand Down Expand Up @@ -1491,8 +1493,8 @@ def check_hash_conficts(latest_hash):

if self._origin is None or latest_hash != self._origin.top_hash:
raise QuiltConflictException(
f"Package with hash {latest_hash} already exists at the destination; "
f"expected {None if self._origin is None else self._origin.top_hash}. "
f"Package with hash {latest_hash!r} already exists at the destination; "
f"expected {None if self._origin is None else self._origin.top_hash!r}. "
"Use force=True (Python) or --force (CLI) to overwrite."
)

Expand Down
4 changes: 1 addition & 3 deletions api/python/quilt3/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ def __init__(self, bucket, path, version_id):
assert version_id is None, "Local keys cannot have a version ID"
if os.name == 'nt':
assert '\\' not in path, "Paths must use / as a separator"
else:
assert not path.startswith('/'), "S3 paths must not start with '/'"

self.bucket = bucket
self.path = path
Expand Down Expand Up @@ -231,7 +229,7 @@ def __str__(self):
params = {}
else:
params = {'versionId': self.version_id}
return urlunparse(('s3', self.bucket, quote(self.path), None, urlencode(params), None))
return urlunparse(('s3', self.bucket, quote('/' + self.path), None, urlencode(params), None))


def fix_url(url):
Expand Down
Loading

0 comments on commit 2d21857

Please sign in to comment.