-
Notifications
You must be signed in to change notification settings - Fork 59
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.
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
browser: parse browser history using browserexport #216
browser: parse browser history using browserexport #216
Conversation
related #118 |
not as efficient as this, just does a hash on the backed up databases |
ah yeah -- that solution was pretty much messed up anyway, need to think how to support that sort of stuff with cachew.. |
thanks! |
|
||
|
||
def _cachew_depends_on() -> List[str]: | ||
return [str(f) for f in inputs()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seanbreckenridge do you remember why are we doing it here instead of just passing depends_on=inputs
to cachew? just tried it and seems to work 🤔 maybe cachew didn't support it back then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I dont remember exactly why but I think it had to do with testability/name clashes based on a different name for cachew.
I dont think I fully understood it at the time (may still not), I had this helper type InputSource
so that mypy was happy:
https://github.com/seanbreckenridge/HPI/blob/master/my/utils/input_source.py
So that I could set the default for the from_paths
in the function signature itself, but still allow the user to override by passing a function which returned a list of paths as inputs:
Something like:
from more_itertools import ilen
from my.bash import history, inputs
print(ilen(history(from_paths=lambda: tuple(inputs()[0:1]))))
would sort of obviously invalidate the cache, but yeah...
python3 test.py
[INFO 2023-11-06 16:31:38,732 my.bash __init__.py:784 ] [my.bash:history] wrote 59 objects to cachew (sqlite:/home/sean/.cache/cachew/my.bash:history)
59
Not really sure how important that is, someone could always override the inputs by patching their config (having an example of this in the docs might be nice?), I havent really ever found myself specifying a custom function to the inputs so its probably not a feature worthwhile keeping around
you can change it to whatever makes sense for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cant reproduce the error anymore but I remember having to rename stuff here specifically:
from from_paths
to for_paths
in the _cache_depends_on
helper function. otherwise there was some hard to debug error when passing an anonymous lambda function I couldnt figure out. doesnt seem to happen anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah good point about documenting patching the config! I was kinda reluctant because it's kinda hacky, but seems like it's working as intended so far
there are some examples scattered around
https://github.com/karlicoss/promnesia/blob/master/src/promnesia/sources/browser.py#L53-L67
from my modules:
https://github.com/seanbreckenridge/HPI/blob/1fba8ccf2fe93d67198aaa77355cac5f5910e7ac/my/browser/export.py
moved the nested configuration blocks above the code generation since it doesnt work properly