FEAT(index) add --sources & --overwrite to CLI to partially update db from config sources

[ankostis]

Fixes #20

new --sources option(s)

• args can be both Source.name & Source's positional index.
• useful with dynamic partial updates, below (now the default), so that the original config file ca be used.

new --overwrite option

• Added a single CLI option almost-described in the 2nd alternative case explained below,
  due to simplicity. if missing, defaults to "dynamic partial updates" as described in this and this.
• Dropped PROMNESIA_INDEX_POLICY env-var, since it is the default behavior now.
• Function defaults for --overwrite are False, as suggested in the comment below.
• Both promnesia index & promnesia demo sub-commands have been updated.
• All update/overwrite decision logic moved to promnesia/__main__.py.:

Various

• Fix: create db-output directory in advance.

[ankostis]

I'd like to add here more commits, to implement a new CLI option --update --overwrite <policy> from #20, where <policy> can be:

• yes: delete & recreate promnesia db with newly indexed visits
• no: keep existing visits and merge new ones collected
• if_partial: (default when option is missing) update when --sources has been given, overwrite otherwise

Also documentation is missing (and a CHANGES.md would be good to have file).

[ankostis]

Another alternative would be to add just 2 mutual-exclusive options: --update & --overwrite, which would be terser for users. But the previous syntax is more extendable, if in the future a new policy appears.

If will implement this, and await feedback.

[ankostis]

Implemented the 2nd alternative (and updated OP above with the new feature).

[ankostis]

This PR is now only missing the docs & examples, and at the moment has taken the following design choices:

Added --update / --overwrite mutual exclusive options

• Added the CLI options described in the 2nd alternative case explained below,
  due to simplicity.
• The precedence for deciding whether to update or overwrite:
  • env-var PROMNESIA_INDEX_POLICY (now strick-check its value is one of (update|overwrite), case-insensitevely)
  • --update / --overwrite from CLI
  • --sources given? (--update conveyed)
  • --overwrite as fall-back (changing this would be a breaking change).
• Function defaults in the new code are False, as suggested in this comment of #20.
• Both promnesia index & promnesia demo sub-commands have been updated.
• Env-var now checks its value one of (update|overwrite).
• All update/overwrite decision logic moved to main.

[karlicoss]

Hm, CI fails (for the context, this

promnesia/.github/workflows/main.yml

Line 40 in e44a2bd

- run: bash scripts/ci/run

is the script that github actions are running), but in essence, you can reproduce it localy via CI=true tox.

You can also run specific parts of tox as for example CI=true tox -e tests
https://github.com/karlicoss/promnesia/blob/master/tox.ini#L14

But also of course running tests directly with pytest would work too, guess you'll figure this out. But tox is the closest approximation to CI.

[karlicoss]

PROMNESIA_INDEX_POLICY

To be honest, I think it would make sense to deprecate it (not a major violation of backwards compatibility, doubt many people used it either). But maybe nice to add a warning.warn to start with.

[karlicoss]

Thanks! Left some comments..
Btw think I noticed an issue recently -- I increased the frequency of updates and sometimes noticed that backend is unresponsinve (likely during DB updates). Not directly related to this PR, just something to keep in mind for the future (since --update makes sense as default)

[ankostis]

A kind request, when it's time to merge (not yet), i suggest to rebase this PR - not to squash it,
in order to preserve the commits herein, since they are articulated and isolated as "features", well described accordingly on each commit-message.

[karlicoss]

i suggest to rebase this PR - not to squash it,

Yes! I have a similar workflow, and happy to do that when people maintain clean history. I think I squashed your first PR since it had lots of 'fix' commits, but the other one I just rebased as is.

[ankostis]

More work...

To be honest, I think it would make sense to deprecate PROMNESIA_INDEX_POLICY (not a major violation of backwards compatibility, doubt many people used it either). But maybe nice to add a warning.warn to start with.
...
(since --update makes sense as default)

• ok, 2d5c3c4 dropped --update option (only --overwrite remains), as "update" is now the default behavior, and
• therefore removed that env-var completely, since if anyone had used PROMNESIA_INDEX_POLICY=update,
  won't notice any difference
  (assuming nobody used PROMNESIA_INDEX_POLICY=no, and decided that notifying user about this experimental feature being dropped, didn't worth its weight).
• Now --sources option is uncoupled from --overwrite (much simpler CLI to grasp).
• OP has been updated to reflect the final functionality of this PR.

Still owing ...

• Some test-cases on the new functionality,
  if only i could make them all run (see my question about private module, above)??
• Also please consider RFE: format(black) source code to facilitate new PRs #213 (black-en sources) and adding CHANGES.md.

[karlicoss]

For #213 -- will respond there
For changelog -- yeah I agree. Let's discuss at #215

[karlicoss]

Hooray, thanks!

Sorry it's taken a while, I guess todos for me which would make it quicker next time:

• make github actions run on pull requests & forks
• add a section to development.org about running tests (tox/mypy/ -k argument) & basic explanation of what's in each of them (maybe even docstring is good enough) + mention submodules update command explicitly
• (maybe) get rid of circleci in favor of github actions, just less external dependencies and makes it easier to run on forks as well

I'll look at it after I get some sleep (crazy schedule)

Other todos from the discussions:

• add meaningful test for update functionality (I can do it and share the PR?)
  in particular, there might be a minor race condition (database unresponsive during the update), would be nice to cover it with regression test
• add some basic cli tests
• think about unifying cli commands in the future, since they all kind of do similar things

Not sure if forgot anything?

[karlicoss]

Done the CI updates & developer info! #217 keeping circle though, since it's still useful because of systemd test.

Also if you go to “Actions” tab on your promnesia fork and click “I understand my workflows, go ahead and enable them”, the pipelines should start running for you even before you PR, so hopefully will make it all quicker next time.