Bug 14674 - koha-create should set upload_path in koha-conf.xml
Summary: koha-create should set upload_path in koha-conf.xml
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Whiteboard:
Keywords: rel_3_22_candidate
Depends on:
Blocks:
 
Reported: 2015-08-11 16:02 UTC by Tomás Cohen Arazi
Modified: 2016-06-21 21:39 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14674: koha-create should populate upload_path (6.66 KB, patch)
2015-10-20 14:41 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14674: koha-create should populate upload_path (6.82 KB, patch)
2015-10-23 07:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14674: [QA Follow-up] Typos and textual adjustment (2.50 KB, patch)
2015-10-23 07:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14674: koha-create should populate upload_path (6.84 KB, patch)
2015-10-23 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14674: [QA Follow-up] Typos and textual adjustment (2.52 KB, patch)
2015-10-23 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2015-08-11 16:02:22 UTC

    
Comment 1 Marcel de Rooy 2015-10-18 13:15:14 UTC
What about /usr/share/koha_$instance_upload ?
Comment 2 Marcel de Rooy 2015-10-18 13:16:44 UTC
Note that it would preferably be a folder that is NOT shared between instances. For simplicity I would recommend that each instance, which has its own upload table, also has its own upload folder.
Comment 3 Tomás Cohen Arazi 2015-10-18 17:34:23 UTC
In /var/lib/koha/instace/upload_dir please ☺
Comment 4 Robin Sheat 2015-10-18 23:12:39 UTC
(In reply to Marcel de Rooy from comment #1)
> What about /usr/share/koha_$instance_upload ?

Writeable stuff under /usr/share is very non-standard. On larger unixy systems, it's not uncommon for that to be read-only.

This is what /var is for. My suggestion would simply be /var/lib/koha/[instance]/uploads
Comment 5 Tomás Cohen Arazi 2015-10-20 14:41:13 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2015-10-21 11:36:49 UTC
debian/scripts/koha-create: line 294: local: `=upload21': not a valid identifier
Trivial typo in the routine set_upload_path: remove the $ from instance

sed: -e expression #26, char 20: unknown option to `s'
Not sure why this comes up
But it appears to be something with the new line:
        -e "s/__UPLOAD_PATH__/$UPLOAD_PATH/g" \
Comment 7 Robin Sheat 2015-10-21 12:04:31 UTC
(In reply to Marcel de Rooy from comment #6)
> debian/scripts/koha-create: line 294: local: `=upload21': not a valid
> identifier
> Trivial typo in the routine set_upload_path: remove the $ from instance
> 
> sed: -e expression #26, char 20: unknown option to `s'
> Not sure why this comes up
> But it appears to be something with the new line:
>         -e "s/__UPLOAD_PATH__/$UPLOAD_PATH/g" \

I bet that the / in the path are confusing sed.
Comment 8 Marcel de Rooy 2015-10-21 12:07:02 UTC
(In reply to Robin Sheat from comment #7)
> I bet that the / in the path are confusing sed.

Probably.
Comment 9 Marcel de Rooy 2015-10-23 07:58:45 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2015-10-23 07:58:49 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-10-23 11:19:50 UTC
Created attachment 43818 [details] [review]
Bug 14674: koha-create should populate upload_path

This patch makes koha-create and koha-create-dirs aware of the new upload_path
configuration entry.

It defaults to /var/lib/koha/<instance>/uploads as proposed by Robin but lets the
user specify its own directory, using the --upload-path option switch that is
added by this patch.

koha-create-dirs is tweaked so it also creates this new directory.

The docs are updated accordingly.

To test:
- Apply the patch, have a packages setup (either by grabbing the relevant files [1]
  or by creating your own package).
- Run koha-create --create-db instance
=> SUCCESS: /var/lib/koha/instance/uploads directory is created
=> SUCCESS: /etc/koha/sites/instance/koha-config.xml has upload_path set correctly
- Create a new instance using the --upload-path making it point to whatever you want
=> SUCCESS: koha-conf.xml points to your chosen path
- Sign off :-D

Regards

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Does not work in its current state. Needs a follow-up.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2015-10-23 11:19:55 UTC
Created attachment 43819 [details] [review]
Bug 14674: [QA Follow-up] Typos and textual adjustment

This patch does:

[1] Resolve error [koha-create: line 294: local: `=upload21': not a valid
    identifier] by replacing $instance by instance.
[2] Resolved typo for 'specify'.
[3] Resolve error: [sed: -e expression #26, char 20: unknown option to `s']
    by replacing the slash in the sed line by a #. This makes sed not
    stumble over the slashes in the upload path.
[4] Added the aspect of permanent storage in the wording (as opposed to
    the storage of temporary uploads in /tmp or similar).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified that koha-create now inserts the right path in koha-conf.xml.
Verified that koha-create-dirs created the new uploads directory.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Tomás Cohen Arazi 2015-10-23 13:16:13 UTC
Patches pushed to master.

Thanks for the followup, Marcel!