Modify idx location #6
Conversation
paperwork_backend/docsearch.py
Outdated
| @@ -263,17 +263,23 @@ def __init__(self, rootdir, indexdir=None, language=None, | |||
| self.index = PaperworkIndexClient() | |||
|
|
|||
| self.fs = fs.GioFileSystem() | |||
| self.rootdir = self.fs.safe(rootdir) | |||
|
|
|||
| self.rootdir = self.fs.unsafe(rootdir) | |||
There was a problem hiding this comment.
Will break if rootdir (the work directory) is not on a local path (Samba GVfs or SSH GVfs for instance)
| @@ -139,6 +141,9 @@ def __init__(self): | |||
| 'ocr_lang': PaperworkSetting( | |||
| "OCR", "Lang", get_default_ocr_lang | |||
| ), | |||
| 'index_in_workdir': PaperworkSetting( | |||
There was a problem hiding this comment.
You can make things a little bit simpler:
'index_in_workdir': PaperworkSetting(
"Global", "index_in_workdir", lambda: False, paperwork_cfg_boolean
),(see frontend:src/paperwork/frontend/util/config.py for examples)
paperwork_cfg_boolean will take care of converting for you the read value from the configuration to a boolean.
paperwork_backend/docsearch.py
Outdated
|
|
||
| indexdir = os.path.join(indexdir, "index") | ||
|
|
||
| if self.rootdir is not None and index_in_workdir.lower() == 'true': |
There was a problem hiding this comment.
-
No need to check
self.rootdir is not Nonehere. If it would beNone, the call toself.fs.unsafe(rootdir)would raise an Exception -
index_in_workdir.lower() == 'true'will break with the default value passed of the constructor (__init__(..., index_in_workdir=None, ...)). I think it would be best if the constructor takes a boolean (letting the caller do the conversion if required), with a default value ofFalse.
paperwork_backend/docsearch.py
Outdated
|
|
||
| else: | ||
|
|
||
| if indexdir is None: |
There was a problem hiding this comment.
the if and the else above it can be merged --> elif
paperwork_backend/docsearch.py
Outdated
| @@ -252,7 +252,7 @@ class DocSearch(object): | |||
| LABEL_STEP_UPDATING = "label updating" | |||
| LABEL_STEP_DESTROYING = "label deletion" | |||
|
|
|||
| def __init__(self, rootdir, indexdir=None, language=None, | |||
| def __init__(self, rootdir, indexdir=None, index_in_workdir=None, language=None, | |||
There was a problem hiding this comment.
Can you add this argument at the end please ? So it doesn't break the Flatpak build if it is built before the corresponding changes in the frontend are merged.
paperwork_backend/shell.py
Outdated
| @@ -50,7 +50,7 @@ def get_docsearch(): | |||
|
|
|||
| verbose("Work directory: {}".format(pconfig.settings['workdir'].value)) | |||
|
|
|||
| dsearch = docsearch.DocSearch(pconfig.settings['workdir'].value) | |||
| dsearch = docsearch.DocSearch(pconfig.settings['workdir'].value, index_in_workdir = pconfig.settings['index_in_workdir'].value) | |||
There was a problem hiding this comment.
PEP8: No spaces around the '=' in this case.
|
Thanks for the corrections, @jflesch . Can you take a look at the changes I just committed and see if they are satisfactory? |
paperwork_backend/docsearch.py
Outdated
| localdir = os.path.expanduser("~/.local") | ||
| if indexdir is None: | ||
|
|
||
| if index_in_workdir == True: |
There was a problem hiding this comment.
Suggestion: shorter: if index_in_workdir:
paperwork_backend/docsearch.py
Outdated
| if indexdir is None: | ||
|
|
||
| if index_in_workdir == True: | ||
| self.rootdir = self.fs.unsafe(rootdir) |
There was a problem hiding this comment.
One minor change and you're good to go:
With your current changes, when index_in_workdir is False, self.rootdir is an URI. When index_in_workdir is True, self.rootdir is a Unix path. Having 2 different types of values for a single attribute can be really bug-prone.
I would prefer that self.rootdir always remain an URI.
Here it can be simply fixed by not changing self.rootdir ; with something like:
if index_in_workdir:
base_data_dir = self.fs.unsafe(rootdir)
localdir = base_data_dir
indexdir = os.path.join(base_data_dir, "index")
|
I've merged back the backend part of Paperwork in the same Git repository repository than the frontend. Now everything is again in https://github.com/openpaperwork/paperwork/ . I'll archive this repository. You should be able to just re-apply your changes as-is. Nothing in the code has changed (yet ;). |
|
This repository has been merged with the one of the frontend to make maintenance and continous integration easier. This repository has therefore been archived. I'll have to ask you to open a new PR against https://github.com/openpaperwork/paperwork/tree/develop , sorry. |
|
Also, please don't send PR to include new features into 'master'. They go in the branch 'develop'. |
|
I also have to ask you to not put '.' as commit message ... Please use |
These are the modifications I created to make the index reside with the documents.