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.

Sign up for GitHub

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

Jump to bottom

Exclude fsspec.open_kwargs["client_kwargs"] from pattern hash computation #444

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Exclude fsspec.open_kwargs["client_kwargs"] from pattern hash computation #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
226c2df
Exclude `fsspec.open_kwargs["client_kwargs"]` from recipe hash comput…
derekocallaghan Nov 30, 2022
451de5f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 30, 2022
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
10 changes: 9 additions & 1 deletion pangeo_forge_recipes/patterns.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,15 @@ def pattern_blockchain(pattern: FilePattern) -> List[bytes]:
# index:filepath pairs yielded by iterating over ``.items()``. if these pairs are generated in
# a different way in the future, we ultimately don't care.
root = {
"fsspec_open_kwargs": pattern.fsspec_open_kwargs,
"fsspec_open_kwargs": {
k: v
for k, v in pattern.fsspec_open_kwargs.items()
# `client_kwargs` is excluded because it can contain
# hard-to-hash objects, e.g. aiohttp.ClientTimeout.
# This avoids subsequent errors in
# `serialization.either_encode_or_hash()`.
if k != "client_kwargs"
Copy link

Choose a reason for hiding this comment

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

Could there be the ssl keyword included in this filtering? It causes similar troubles during hashing and my guess is that it's a similar issue: #418 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may be easiest to exclude fsspec_open_kwargs itself from the hash computation, rather than filtering a key list containing client_kwargs etc. @cisaacstern, what do you think?

I had a quick look at the stack trace in #418 (comment) and it looks like a separate problem related to pickle. The expected error associated with this PR issue should be raised in pangeo-forge-recipes.serialization.either_encode_or_hash():

pangeo-forge-recipes/pangeo_forge_recipes/serialization.py

Line 25 in fe5d1f7

raise TypeError(f"object of type {type(obj).__name__} not serializable")

e.g.

Input In [5], in either_encode_or_hash(obj)
     13 elif isinstance(obj, bytes):
     14     return obj.hex()
---> 15 raise TypeError(f"object of type {type(obj).__name__} not serializable")

TypeError: object of type ClientTimeout not serializable

},
"query_string_secrets": pattern.query_string_secrets,
"file_type": pattern.file_type,
"nitems_per_file": {
Expand Down
32 changes: 31 additions & 1 deletion tests/test_serialization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from dataclasses import asdict, dataclass, field
from dataclasses import asdict, dataclass, field, replace
from datetime import datetime, timedelta
from typing import Optional

import aiohttp
import pandas as pd
import pytest
from fsspec.implementations.local import LocalFileSystem
Expand Down Expand Up @@ -102,6 +103,35 @@ def test_recipe_sha256_hash_exclude(base_pattern, recipe_cls, tmpdir_factory):
assert recipe_0.sha256 == recipe_1.sha256


@pytest.mark.parametrize("recipe_cls", [XarrayZarrRecipe, HDFReferenceRecipe])
def test_recipe_sha256_hash_exclude_fsspec_open_kwargs(base_pattern, recipe_cls, tmpdir_factory):
recipe_0 = recipe_cls(base_pattern)
recipe_1 = recipe_cls(base_pattern)

assert recipe_0.sha256 == recipe_1.sha256

# Specify fsspec_open_kwargs that will be excluded,
# e.g. client_kwargs containing certain aiohttp objects
# that can't be easily hashed. This avoids subsequent errors in
# `serialization.either_encode_or_hash()`.
def aio_file_pattern(client_kwargs):
return FilePattern(
base_pattern.format_function,
base_pattern.combine_dims[0],
fsspec_open_kwargs={"client_kwargs": client_kwargs},
)

# Recipe hash computation, including file pattern computation, occurs with replace()
replace(
recipe_1,
file_pattern=aio_file_pattern({"auth": aiohttp.BasicAuth(login="test", password="test")}),
)
assert recipe_0.sha256 == recipe_1.sha256

replace(recipe_1, file_pattern=aio_file_pattern({"timeout": aiohttp.ClientTimeout(total=1800)}))
assert recipe_0.sha256 == recipe_1.sha256


@pytest.mark.parametrize(
"kwargs",
[
Expand Down