Skip to content

Commit

Permalink
Merge branch 'dev' into feature/observability
Browse files Browse the repository at this point in the history
  • Loading branch information
collindutter authored Jul 17, 2024
2 parents 9bdf049 + 8026409 commit b18e54a
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/actions/init-environment/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ runs:

- name: Activate venv
run: |
source .venv/bin/activate
source $VENV
echo PATH=$PATH >> $GITHUB_ENV
shell: bash

Expand Down
22 changes: 21 additions & 1 deletion .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ concurrency:
cancel-in-progress: true

jobs:
test:
test-ubuntu:
runs-on: ubuntu-latest
strategy:
fail-fast: false
Expand All @@ -24,3 +24,23 @@ jobs:
uses: ./.github/actions/init-environment
- name: Run unit tests
run: make test/unit
test-windows:
runs-on: windows-latest
strategy:
fail-fast: false
matrix:
python-version: ["3.9"]
defaults:
run:
shell: bash
steps:
- name: Checkout actions
uses: actions/checkout@v3
- name: Init environment
uses: ./.github/actions/init-environment
- name: Run unit tests
# TODO: make test/unit fails with the following error:
# process_begin: CreateProcess(NULL, poetry run pytest -n auto tests/unit, ...) failed.
# make (e=2): The system cannot find the file specified.
# make: *** [Makefile:24: test/unit] Error 2
run: poetry run pytest -n auto tests/unit
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed
- Parameter `count` for `QdrantVectorStoreDriver.query` now optional as per documentation.
- Path issues on Windows with `LocalFileManagerDriver` and `AmazonS3FileManagerDriver`.

## [0.28.2] - 2024-07-12
### Fixed
Expand Down
27 changes: 22 additions & 5 deletions griptape/drivers/file_manager/amazon_s3_file_manager_driver.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from __future__ import annotations

import os
from pathlib import Path
from typing import TYPE_CHECKING, Any

from attrs import Attribute, Factory, define, field
Expand Down Expand Up @@ -33,7 +31,7 @@ class AmazonS3FileManagerDriver(BaseFileManagerDriver):

@workdir.validator # pyright: ignore[reportAttributeAccessIssue]
def validate_workdir(self, _: Attribute, workdir: str) -> None:
if not Path(workdir).is_absolute():
if not workdir.startswith("/"):
raise ValueError("Workdir must be an absolute path")

def try_list_files(self, path: str) -> list[str]:
Expand Down Expand Up @@ -70,11 +68,13 @@ def try_save_file(self, path: str, value: bytes) -> None:

def _to_full_key(self, path: str) -> str:
path = path.lstrip("/")
full_key = os.path.join(self.workdir, path)
full_key = f"{self.workdir}/{path}"
# Need to keep the trailing slash if it was there,
# because it means the path is a directory.
ended_with_slash = path.endswith("/")
full_key = os.path.normpath(full_key)

full_key = self._normpath(full_key)

if ended_with_slash:
full_key += "/"
return full_key.lstrip("/")
Expand Down Expand Up @@ -126,3 +126,20 @@ def _is_a_directory(self, full_key: str) -> bool:
raise e

return False

def _normpath(self, path: str) -> str:
unix_path = path.replace("\\", "/")
parts = unix_path.split("/")
stack = []

for part in parts:
if part == "" or part == ".":
continue
if part == "..":
if stack:
stack.pop()
else:
stack.append(part)

normalized_path = "/".join(stack)
return normalized_path
5 changes: 4 additions & 1 deletion tests/unit/artifacts/test_blob_artifact.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import base64
import os

import pytest

Expand Down Expand Up @@ -32,7 +33,9 @@ def test_to_dict(self):
assert BlobArtifact(b"foobar", name="foobar.txt", dir_name="foo").to_dict()["name"] == "foobar.txt"

def test_full_path_with_path(self):
assert BlobArtifact(b"foobar", name="foobar.txt", dir_name="foo").full_path == "foo/foobar.txt"
assert BlobArtifact(b"foobar", name="foobar.txt", dir_name="foo").full_path == os.path.normpath(
"foo/foobar.txt"
)

def test_full_path_without_path(self):
assert BlobArtifact(b"foobar", name="foobar.txt").full_path == "foobar.txt"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ def test_save_file(self, workdir, path, content, driver, get_s3_value):

assert isinstance(result, InfoArtifact)
assert result.value == "Successfully saved file"
expected_s3_key = os.path.join(workdir, path).lstrip("/")
expected_s3_key = f"{workdir}/{path}".lstrip("/")
content_str = content if isinstance(content, str) else content.decode()
assert get_s3_value(expected_s3_key) == content_str

Expand Down Expand Up @@ -258,11 +258,11 @@ def test_save_file_with_encoding(self, session, bucket, get_s3_value):
driver = AmazonS3FileManagerDriver(
session=session, bucket=bucket, default_loader=TextLoader(encoding="utf-8"), loaders={}, workdir=workdir
)
path = os.path.join("test", "foobar.txt")
path = "test/foobar.txt"

result = driver.save_file(path, "foobar")

expected_s3_key = os.path.join(workdir, path).lstrip("/")
expected_s3_key = f"{workdir}/{path}".lstrip("/")
assert get_s3_value(expected_s3_key) == "foobar"
assert result.value == "Successfully saved file"

Expand All @@ -271,18 +271,18 @@ def test_save_and_load_file_with_encoding(self, session, bucket, get_s3_value):
driver = AmazonS3FileManagerDriver(
session=session, bucket=bucket, loaders={"txt": TextLoader(encoding="ascii")}, workdir=workdir
)
path = os.path.join("test", "foobar.txt")
path = "test/foobar.txt"

result = driver.save_file(path, "foobar")

expected_s3_key = os.path.join(workdir, path).lstrip("/")
expected_s3_key = f"{workdir}/{path}".lstrip("/")
assert get_s3_value(expected_s3_key) == "foobar"
assert result.value == "Successfully saved file"

driver = AmazonS3FileManagerDriver(
session=session, bucket=bucket, default_loader=TextLoader(encoding="ascii"), loaders={}, workdir=workdir
)
path = os.path.join("test", "foobar.txt")
path = "test/foobar.txt"

result = driver.load_file(path)

Expand Down
21 changes: 9 additions & 12 deletions tests/unit/drivers/file_manager/test_local_file_manager_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def test_validate_workdir(self):
)
def test_list_files(self, workdir, path, expected, temp_dir, driver):
# Treat the workdir as an absolute path, but modify it to be relative to the temp_dir.
driver.workdir = os.path.join(temp_dir, os.path.abspath(workdir).lstrip("/"))
driver.workdir = self._to_driver_workdir(temp_dir, workdir)

artifact = driver.list_files(path)

Expand All @@ -121,7 +121,7 @@ def test_list_files(self, workdir, path, expected, temp_dir, driver):
)
def test_list_files_failure(self, workdir, path, expected, temp_dir, driver):
# Treat the workdir as an absolute path, but modify it to be relative to the temp_dir.
driver.workdir = os.path.join(temp_dir, os.path.abspath(workdir).lstrip("/"))
driver.workdir = self._to_driver_workdir(temp_dir, workdir)

artifact = driver.list_files(path)

Expand Down Expand Up @@ -155,7 +155,7 @@ def test_load_file(self, driver: LocalFileManagerDriver):
)
def test_load_file_failure(self, workdir, path, expected, temp_dir, driver):
# Treat the workdir as an absolute path, but modify it to be relative to the temp_dir.
driver.workdir = os.path.join(temp_dir, os.path.abspath(workdir).lstrip("/"))
driver.workdir = self._to_driver_workdir(temp_dir, workdir)

artifact = driver.load_file(path)

Expand Down Expand Up @@ -194,7 +194,7 @@ def test_load_file_with_encoding_failure(self):
)
def test_save_file(self, workdir, path, content, temp_dir, driver):
# Treat the workdir as an absolute path, but modify it to be relative to the temp_dir.
driver.workdir = os.path.join(temp_dir, os.path.abspath(workdir).lstrip("/"))
driver.workdir = self._to_driver_workdir(temp_dir, workdir)

result = driver.save_file(path, content)

Expand Down Expand Up @@ -223,7 +223,7 @@ def test_save_file(self, workdir, path, content, temp_dir, driver):
)
def test_save_file_failure(self, workdir, path, expected, temp_dir, driver):
# Treat the workdir as an absolute path, but modify it to be relative to the temp_dir.
driver.workdir = os.path.join(temp_dir, os.path.abspath(workdir).lstrip("/"))
driver.workdir = self._to_driver_workdir(temp_dir, workdir)

artifact = driver.save_file(path, "foobar")

Expand Down Expand Up @@ -251,10 +251,7 @@ def test_save_and_load_file_with_encoding(self, temp_dir):
assert len(result.value) == 1
assert isinstance(result.value[0], TextArtifact)

def test_chrdir_getcwd(self, temp_dir):
os.chdir(temp_dir)
file_manager_1 = LocalFileManagerDriver()
assert file_manager_1.workdir.endswith(temp_dir)
os.chdir("/tmp")
file_manager_2 = LocalFileManagerDriver()
assert file_manager_2.workdir.endswith("/tmp")
def _to_driver_workdir(self, temp_dir, workdir):
# Treat the workdir as an absolute path, but modify it to be relative to the temp_dir.
root_relative_parts = Path(os.path.abspath(workdir)).parts[1:]
return os.path.join(temp_dir, Path(*root_relative_parts))
2 changes: 1 addition & 1 deletion tests/unit/utils/test_command_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@

class TestCommandRunner:
def test_run(self):
assert CommandRunner().run("echo 'test'").value == "test"
assert "test" in CommandRunner().run("echo 'test'").value
2 changes: 0 additions & 2 deletions tests/unit/utils/test_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ def test_load_file(self):
file = utils.load_file(os.path.join(dirname, "../../resources/foobar-many.txt"))

assert file.decode("utf-8").startswith("foobar foobar foobar")
assert len(file.decode("utf-8")) == 4563

def test_load_files(self):
dirname = os.path.dirname(__file__)
Expand All @@ -24,7 +23,6 @@ def test_load_files(self):
assert len(files) == 2

test_file = files[utils.str_to_hash(sources[0])]
assert len(test_file) == 4563
assert test_file.decode("utf-8").startswith("foobar foobar foobar")

small_file = files[utils.str_to_hash(sources[2])]
Expand Down

0 comments on commit b18e54a

Please sign in to comment.