Move the core logic from Run.log() to AttributeStore
#96
Conversation
kgodlewski
force-pushed
the
kg/refactor-attribute-store
branch
from
0c26195 to
70fa6be
Compare
| """ | ||
|
|
||
| path = path.strip() | ||
| if path in ("", "/"): | ||
| if path.strip() in ("", "/"): |
There was a problem hiding this comment.
Hmm, since you're not assigning the stripped path back to path, if the path has some whitespace, it will stay and break the 2 checks following immediately below.
E.g.
previously " /path/to/sth/ " -> strip() -> "/path/to/sth/" -> path[1:] -> "path/to/sth/" -> error because it ends with /
now " /path/to/sth/ " -> strip() is not saved -> " /path/to/sth/ " -> path[1:] ignored bc path starts with " " -> " /path/to/sth/ " -> no error because it ends with " "...
src/neptune_scale/api/attribute.py
Outdated
| raise ValueError(f"Invalid path: `{path}`. Path components must not be empty.") | ||
|
|
||
| if path[0].lstrip() != path[0] or path[-1].rstrip() != path[-1]: |
There was a problem hiding this comment.
hmm, path[0] and path[-1] are substrings of size 0 or 1, so there's no difference between lstrip() and rstrip()?
There was a problem hiding this comment.
Yeah I think I missed a split() or two
|
Is it alright that such a major replacement has no changes in tests/? Does not incite confidence.. |
`AttributeStore.log()` is now responsible for feeding data to the `OperationsQueue`
kgodlewski
force-pushed
the
kg/refactor-attribute-store
branch
from
70fa6be to
3406313
Compare
kgodlewski
force-pushed
the
kg/refactor-attribute-store
branch
from
0bf6895 to
f933b08
Compare
AttributeStore.log()is now responsible for feeding data to theOperationsQueue.Some additional cleanups are there as well.