|
Description
Hugo Agud
2018-11-28 15:40:05 UTC
Created attachment 88598 [details] [review] Bug 21903: koha-dump can export uploaded and temporary uploaded files Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Created attachment 88867 [details] [review] Bug 21903: koha-dump can export uploaded and temporary uploaded files Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. I am testing the patch, and it is not working, it is not accepting the options, it only works the koha-dump <<instance>> (In reply to Hugo Agud from comment #3) > I am testing the patch, and it is not working, it is not accepting the > options, it only works the koha-dump <<instance>> Hi Hugo, what was the command that didn't work and what the error you received? I think you always need to give the instande + options you want to use. I have tested all options sudo koha-dump --uploaded_files kohadev sudo koha-dump kohadev --uploaded_files Dumping Koha site kohadev: * DB to /var/spool/koha/kohadev/kohadev-2019-08-20.sql.gz * configs, logs to /var/spool/koha/kohadev/kohadev-2019-08-20.tar.gz Done. grep: unrecognized option '--uploaded_files' Usage: grep [OPTION]... PATTERN [FILE]... Try 'grep --help' for more information. Error: Invalid instance name --uploaded_files perhaps is the path where I should launch it, /home/vagrant/kohaclone? or another? I don't know about vagrant, but on a regular debian packages-based install, once the following files are patched: /usr/sbin/koha-dump /usr/share/koha/bin/koha-functions.sh I can run the backup using the following command: # koha-dump --uploaded_files koha Dumping Koha site koha: * DB to /var/spool/koha/koha/koha-2019-08-29.sql.gz * configs, files, logs to /var/spool/koha/koha/koha-2019-08-29.tar.gz Done. Notice the difference between our messages: yours: configs, logs to mine: configs, files, logs to The "files" part is added by the patch, so I'm guessing you're executing an unpatched version of koha-dump. Just a note here for those that are using Vagrant installs (kohadevbox) you need to run reset_all for the /usr/sbin/koha-dump and /usr/share/koha/bin/koha-functions.sh to get the updated flags. After that I get matching messages for regular koha-dump: Dumping Koha site kohadev: * DB to /var/spool/koha/kohadev/kohadev-2019-10-02.sql.gz * configs, files, logs to /var/spool/koha/kohadev/kohadev-2019-10-02.tar.gz Done. But at last after I try to run the koha-dump --uploaded_files kohadev (I get) Error: invalid option switch (--uploaded_files) SO maybe I am wrong and this needs to be tested on a regular debian package. (I'll ping Tomas and see if he understands a little more about the vagrant installs). Thanks for the feedback. My bad, I somehow messed up when submitting the patch. It should be ok now. Created attachment 93550 [details] [review] Bug 21903: koha-dump can export uploaded and temporary uploaded files Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Created attachment 93862 [details] [review] Bug 21903: koha-dump can export uploaded and temporary uploaded files Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Signed-off-by: Hugo Agud <hagud@orex.es> Looks like this should be SO. HI, Rebase it for testing. Bug 21903 - koha-dump be able to include koha-upload 93862 - Bug 21903: koha-dump can export uploaded and temporary uploaded files Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 21903: koha-dump can export uploaded and temporary uploaded files Using index info to reconstruct a base tree... M debian/scripts/koha-dump M debian/scripts/koha-functions.sh Falling back to patching base and 3-way merge... Auto-merging debian/scripts/koha-functions.sh Auto-merging debian/scripts/koha-dump CONFLICT (content): Merge conflict in debian/scripts/koha-dump error: Failed to merge in the changes. Patch failed at 0001 Bug 21903: koha-dump can export uploaded and temporary uploaded files The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-21903-koha-dump-can-export-uploaded-and-tempor-jZTdqB.patch Created attachment 126835 [details] [review] Bug 21903: koha-dump can export uploaded and temporary uploaded files Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Signed-off-by: Hugo Agud <hagud@orex.es> This is the rebased version Created attachment 126907 [details] [review] Bug 21903: koha-dump can export uploaded and temporary uploaded files Minor improvment on display. Solve the error message if the folder for uploaded temp files doesn't exist. Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Change message and solve the error message if no temporary folder Created attachment 126908 [details] [review] Bug 21903: (follow-up) koha-dump can export uploaded and temporary uploaded files Minor improvment on display. Solve the error message if the folder for uploaded temp files doesn't exist. Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Change message and solve the error message if no temporary folder Ok, a few comments: - If you add parameters to a koha-* script, you need to add them to the docs in debian/docs/koha-* as well. This is a blocker, so FQA Also - I don't 100% understand how the uploaded tmp files will be differentiated from any other file in tmp. The 'get_tmp_path' function name in koha-functions.sh raised my suspicion, as it doesn't mention 'upload' or 'uploaded'. This could be revisited too. - I have the feeling that in order to make this more useful for production, we could try thinking how this could be used from koha-run-backups. When I added the 'exclude_indexes' option switch to koha-dump, I also added it to koha-run-backups. In this case it is not a flag (yes/no) but a string parameter with a path, so it doesn't sound like a straight-forward addition. But... maybe an option switch to include/not include uploads on the backup? Food for thougth, not a blocker. Created attachment 128732 [details] [review] Bug 21903: (follow-up) update documentation (In reply to Tomás Cohen Arazi from comment #17) > Ok, a few comments: > - If you add parameters to a koha-* script, you need to add them to the docs > in debian/docs/koha-* as well. This is a blocker, so FQA I just submited a patch to fix that point. > Also > - I don't 100% understand how the uploaded tmp files will be differentiated > from any other file in tmp. The 'get_tmp_path' function name in > koha-functions.sh raised my suspicion, as it doesn't mention 'upload' or > 'uploaded'. This could be revisited too. The function 'get_tmp_path' get the general tmp files but then we complete the path to the uploaded folder with this line: 'uploaded_temp_files_dir="$tempdir/koha_${name}_upload" (tempdir is set just before with 'get_tmp_path' fuction) > - I have the feeling that in order to make this more useful for production, > we could try thinking how this could be used from koha-run-backups. When I > added the 'exclude_indexes' option switch to koha-dump, I also added it to > koha-run-backups. In this case it is not a flag (yes/no) but a string > parameter with a path, so it doesn't sound like a straight-forward addition. > But... maybe an option switch to include/not include uploads on the backup? > Food for thougth, not a blocker. I agree with you but as it doesn't sone simple, I won't do it here. I opened a new bug (Bug 29738) and I'll will add a patch soon. Not sure I can finish there, but I can at least start? 1) POD (not blocker) + --uploaded_files Include uploaded files. + --uploaded_temp_files Include temporary uploaded files. Does this mean that --uploaded_files includes or excludes the temporary uploaded files? Taking a closer look it looks like temporary and other uploaded files go to different directories, so the first would be excluding temporary files. Just wondering if someone setting this up would be aware - might still be worth adding a little note. 2) What do I need to do to make this work on ktd? I seem to remember having to copy the files somewhere for these changes to take effect. At the moment sudo koha-dump -h doesn't list the new options. Not a reason to fail, but maybe you can help me to test this? Also happy if Tomas wants to finish here. > 2) What do I need to do to make this work on ktd?
running `reset_all` or restarting ktd after applying the patches will make the new files be copied in the right place.
(In reply to Katrin Fischer from comment #20) > 1) POD (not blocker) > > + --uploaded_files Include uploaded files. > + --uploaded_temp_files Include temporary uploaded files. > > Does this mean that --uploaded_files includes or excludes the temporary > uploaded files? > > Taking a closer look it looks like temporary and other uploaded files go to > different directories, so the first would be excluding temporary files. Just > wondering if someone setting this up would be aware - might still be worth > adding a little note. I think that it is clear enough. If you dont know what they are, you wont be specifying these options for koha-dump either. Created attachment 134349 [details] [review] Bug 21903: koha-dump can export uploaded and temporary uploaded files Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 134350 [details] [review] Bug 21903: (follow-up) koha-dump can export uploaded and temporary uploaded files Minor improvment on display. Solve the error message if the folder for uploaded temp files doesn't exist. Sponsored-by: Orex Test plan: - Apply the patch - Try using koha-dump without any option - Try using koha-dump with --uploaded_files - Try using koha-dump with --uploaded_temp_files - Try using koha-dump with both of the options above Expected results: the .tar.gz dump will include uploaded and/or temporary uploaded files if requested. Change message and solve the error message if no temporary folder Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 134351 [details] [review] Bug 21903: (follow-up) update documentation Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 22.11. Nice work everyone, thanks! Enhancement will not be backported to 22.05.x series unless requested It looks like Philip and I were working on the documentation for this one at the same time! Thanks Philip! End result: 1. The package commands page on the Wiki page is updated. 2. Package commands aren't covered in the manual, so no updates required. 3. I updated the installation section of the FAQs page and submitted a merge request. Thanks back to you David for the much more detailed documentation and entry in Installation FAQs! :) |