Fokko commented on code in PR #6392:
URL: https://github.com/apache/iceberg/pull/6392#discussion_r1049285936
##########
python/Makefile:
##########
@@ -26,14 +26,21 @@ lint:
poetry run pre-commit run --all-files
test:
- poetry run coverage run --source=pyiceberg/ -m pytest tests/ -m "not
s3" ${PYTEST_ARGS}
+ poetry run coverage run --source=pyiceberg/ -m pytest tests/ -m "not s3
and not adlfs" ${PYTEST_ARGS}
Review Comment:
I've been looking at this in https://github.com/apache/iceberg/pull/6398/ as
well. I've added:
```python
def pytest_collection_modifyitems(items, config):
for item in items:
if not any(item.iter_markers()):
item.add_marker("unmarked")
```
Which will automatically add the mark `unmarked` to each of the tests that
doesn't have a marked, creating their own group. WDYT of that?
##########
python/tests/conftest.py:
##########
@@ -76,13 +76,30 @@
def pytest_addoption(parser: pytest.Parser) -> None:
+ # S3 options
parser.addoption(
"--s3.endpoint", action="store", default="http://localhost:9000";,
help="The S3 endpoint URL for tests marked as s3"
)
parser.addoption("--s3.access-key-id", action="store", default="admin",
help="The AWS access key ID for tests marked as s3")
parser.addoption(
"--s3.secret-access-key", action="store", default="password",
help="The AWS secret access key ID for tests marked as s3"
)
+ # ADLFS options
+ parser.addoption(
+ "--adlfs.endpoint",
+ action="store",
+ default="http://127.0.0.1:10000";,
+ help="The ADLS endpoint URL for tests marked as adlfs",
+ )
+ parser.addoption(
+ "--adlfs.account-name", action="store", default="devstoreaccount1",
help="The ADLS account key for tests marked as adlfs"
+ )
+ parser.addoption(
+ "--adlfs.account-key",
+ action="store",
+
default="Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==",
Review Comment:
Maybe we should add this Github comment as a code comment, in case we start
doing credential scanning one day.
##########
python/pyiceberg/io/fsspec.py:
##########
@@ -143,8 +153,15 @@ def open(self) -> InputStream:
Returns:
OpenFile: An fsspec compliant file-like object
+
+ Raises:
+ FileNotFoundError: If the file does not exist
"""
- return self._fs.open(self.location, "rb")
+ try:
+ return self._fs.open(self.location, "rb")
+ except FileNotFoundError as e:
+ # To have a consistent error handling experience, make sure
exception contains missing file location.
Review Comment:
This is perfect! We don't want to return implementation-specific errors
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
