Skip to content

Commit

Permalink
Merge branch 'master' of https://github.com/dandi/dandi-cli
Browse files Browse the repository at this point in the history
* 'master' of https://github.com/dandi/dandi-cli:
  Skip cache+symlink test on pre-3.8 Python on Windows
  Fix bug in test_memoize_path
  Run GitHub Actions tests daily
  BF: account for API returning metadata for dandiset now not nested within yet another key
  BF: ensure_datetime for mtime beore taking .timestamp
  BF: always install dateutil and use it by default for parsing ISO datetime strings
  • Loading branch information
yarikoptic committed Sep 4, 2020
2 parents 93f2222 + b042a68 commit 1434966
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 20 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
name: Tests

on:
- push
- pull_request
push:
pull_request:
schedule:
- cron: '0 6 * * *'

jobs:
test:
Expand Down
18 changes: 13 additions & 5 deletions dandi/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
)
from .dandiset import Dandiset
from .exceptions import FailedToConnectError, NotFoundError, UnknownURLError
from .utils import flattened, is_same_time, get_instance
from .utils import ensure_datetime, flattened, is_same_time, get_instance

import humanize
from .support.pyout import naturalsize
Expand Down Expand Up @@ -401,10 +401,18 @@ def download_generator(

# Handle our so special dandiset.yaml
if dandiset and get_metadata:
dandiset_metadata = dandiset.get("metadata", {})
# DANDI API has no versioning yet, and things are in flux.
# It used to have metadata within a key... just in case let's also
# be able to handle "old" style
# TODO: remove when API stabilizes
if (
"identifier" not in dandiset_metadata
and "dandiset" in dandiset_metadata
):
dandiset_metadata = dandiset_metadata["dandiset"]
for resp in _populate_dandiset_yaml(
dandiset_path,
dandiset.get("metadata", {}).get("dandiset", {}),
existing == "overwrite",
dandiset_path, dandiset_metadata, existing == "overwrite"
):
yield dict(path=dandiset_metadata_file, **resp)

Expand Down Expand Up @@ -798,6 +806,6 @@ def _download_file(
# TODO: dissolve attrs and pass specific mtime?
if mtime:
yield {"status": "setting mtime"}
os.utime(path, (time.time(), mtime.timestamp()))
os.utime(path, (time.time(), ensure_datetime(mtime).timestamp()))

yield {"status": "done"}
22 changes: 13 additions & 9 deletions dandi/support/tests/test_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import time

from dandi.support.cache import PersistentCache
from dandi.utils import on_windows

import pytest

Expand Down Expand Up @@ -141,15 +142,18 @@ def check_new_memoread(arg, content, expect_new=False):
check_new_memoread(0, "Content")

# Check that symlinks should be dereferenced
symlink1 = str(tmp_path / (fname + ".link1"))
try:
os.symlink(fname, symlink1)
except OSError:
pass
if op.islink(fname): # hopefully would just skip Windows if not supported
ncalls = len(calls)
assert memoread(symlink1, 0) == "Content"
assert len(calls) == ncalls # no new call
if not on_windows or sys.version_info[:2] >= (3, 8):
# realpath doesn't work right on Windows on pre-3.8 Python, so skip the
# test then.
symlink1 = str(tmp_path / (fname + ".link1"))
try:
os.symlink(fname, symlink1)
except OSError:
pass
if op.islink(symlink1): # hopefully would just skip Windows if not supported
ncalls = len(calls)
assert memoread(symlink1, 0) == "Content"
assert len(calls) == ncalls # no new call

# and if we "clear", would it still work?
cache.clear()
Expand Down
9 changes: 6 additions & 3 deletions dandi/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
from .consts import dandi_instance, known_instances, known_instances_rev
from .exceptions import BadCliVersionError, CliVersionTooOldError

if sys.version_info[:2] < (3, 7):
import dateutil.parser
import dateutil.parser

#
# Additional handlers
Expand Down Expand Up @@ -150,7 +149,11 @@ def ensure_strtime(t, isoformat=True):

def fromisoformat(t, impl=None):
if impl is None:
impl = "datetime" if sys.version_info[:2] >= (3, 7) else "dateutil"
# datetime parser "does not support parsing arbitrary ISO 8601 strings"
# https://docs.python.org/3/library/datetime.html
# In particular it does not parse time zone suffix which was recently
# introduced into datetime's provided by API
impl = "dateutil"
if impl == "datetime":
return datetime.datetime.fromisoformat(t)
elif impl == "dateutil":
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ install_requires =
ruamel.yaml >=0.15, <1
keyring
keyrings.alt
python-dateutil; python_version < "3.7"
python-dateutil
semantic-version
tqdm
typing_extensions; python_version < "3.8"
Expand Down

0 comments on commit 1434966

Please sign in to comment.