Bug 38115 - Add FTP support to export_records.pl
Summary: Add FTP support to export_records.pl
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Jake Deery
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on: 38489
Blocks: 40811
  Show dependency treegraph
 
Reported: 2024-10-08 09:59 UTC by Martin Renvoize (ashimema)
Modified: 2025-09-30 10:41 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 38115: Add destination_server_id to export_records (5.02 KB, patch)
2025-02-27 11:30 UTC, Jake Deery
Details | Diff | Splinter Review
Bug 38115: Add destination_server_id to export_records (5.02 KB, patch)
2025-09-18 19:52 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38115: (follow-up) Update terminology from SFTP/FTP to file transport (4.19 KB, patch)
2025-09-18 19:52 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38115: Add destination_server_id to export_records (5.09 KB, patch)
2025-09-24 06:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38115: (follow-up) Update terminology from SFTP/FTP to file transport (4.26 KB, patch)
2025-09-24 06:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38115: (follow-up) Improve error handling and connection management (4.02 KB, patch)
2025-09-24 06:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2024-10-08 09:59:14 UTC
With an administrative UI for configuring FTP connections coming but bug 35761 it would be nice to add support for using those connections in the misc/export_records.pl script.
Comment 1 Jake Deery 2025-02-27 11:30:07 UTC
Created attachment 178781 [details] [review]
Bug 38115: Add destination_server_id to export_records

This patch adds two new flags to the export_records.pl script.
* --destination_server_id=ID
    This flag will, when passed, upload the file generated by this script to
    the destination server, as configured in the FTP/SFTP Servers settings.
* --delete_local_after_run
    This flag will, when passed, cause the script to unlink (delete) the
    generated marc or CSV file found on the local disk. This is useful for folk
    who want to upload the file, but not necessarily keep a local copy, filling
    up the server's storage medium

TO TEST:

a)  APPLY PATCH, reset_all
b)  Create valid FTP/SFTP Server in Koha settings, ensuring that the server
    you create is valid and passes testing
c)  Run ./misc/export_records.pl with no agruments
    1)  Notice how koha.mrc file is generated
d)  Run ./misc/export_records.pl --destination_server_id=1
    1)  Notice how koha.mrc file is generated
    2)  On your FTP/SFTP Server, notice how koha.mrc is now present in the
        upload directory
e)  Run ./misc/export_records.pl --delete_local_after_run
    1)  Notice how koha.mrc file is NOT generated locally
f)  Run ./misc/export_records.pl --destination_server_id=1 --delete_local_after_run
    1)  Notice how koha.mrc file is NOT generated locally
    2)  On your FTP/SFTP Server, notice how koha.mrc is still present in the
        upload directory AND the file age is fresh
Comment 2 Tomás Cohen Arazi (tcohen) 2025-09-15 15:08:26 UTC
After the changes in bug 39190, terminology should be revisited. And so variable names, etc.
Comment 3 Tomás Cohen Arazi (tcohen) 2025-09-18 19:52:45 UTC
Created attachment 186589 [details] [review]
Bug 38115: Add destination_server_id to export_records

This patch adds two new flags to the export_records.pl script.
* --destination_server_id=ID
    This flag will, when passed, upload the file generated by this script to
    the destination server, as configured in the FTP/SFTP Servers settings.
* --delete_local_after_run
    This flag will, when passed, cause the script to unlink (delete) the
    generated marc or CSV file found on the local disk. This is useful for folk
    who want to upload the file, but not necessarily keep a local copy, filling
    up the server's storage medium

TO TEST:

a)  APPLY PATCH, reset_all
b)  Create valid FTP/SFTP Server in Koha settings, ensuring that the server
    you create is valid and passes testing
c)  Run ./misc/export_records.pl with no agruments
    1)  Notice how koha.mrc file is generated
d)  Run ./misc/export_records.pl --destination_server_id=1
    1)  Notice how koha.mrc file is generated
    2)  On your FTP/SFTP Server, notice how koha.mrc is now present in the
        upload directory
e)  Run ./misc/export_records.pl --delete_local_after_run
    1)  Notice how koha.mrc file is NOT generated locally
f)  Run ./misc/export_records.pl --destination_server_id=1 --delete_local_after_run
    1)  Notice how koha.mrc file is NOT generated locally
    2)  On your FTP/SFTP Server, notice how koha.mrc is still present in the
        upload directory AND the file age is fresh
Comment 4 Tomás Cohen Arazi (tcohen) 2025-09-18 19:52:50 UTC
Created attachment 186590 [details] [review]
Bug 38115: (follow-up) Update terminology from SFTP/FTP to file transport

This follow-up patch updates the terminology in export_records.pl to use
generic 'file transport' language instead of specific 'FTP/SFTP' references,
aligning with the recent renaming of the underlying classes to File::Transports.

Changes:
- Replace $sftp_server variable with $file_transport for clarity
- Update error messages to use 'file transport server' terminology
- Fix incorrect error message for report_id validation
- Update POD documentation to reference 'file transport servers'
- Maintain consistency with the renamed admin interface and API

This ensures the script terminology matches the current class structure
and admin interface naming conventions.
Comment 5 Martin Renvoize (ashimema) 2025-09-24 06:21:59 UTC
Created attachment 186855 [details] [review]
Bug 38115: Add destination_server_id to export_records

This patch adds two new flags to the export_records.pl script.
* --destination_server_id=ID
    This flag will, when passed, upload the file generated by this script to
    the destination server, as configured in the FTP/SFTP Servers settings.
* --delete_local_after_run
    This flag will, when passed, cause the script to unlink (delete) the
    generated marc or CSV file found on the local disk. This is useful for folk
    who want to upload the file, but not necessarily keep a local copy, filling
    up the server's storage medium

TO TEST:

a)  APPLY PATCH, reset_all
b)  Create valid FTP/SFTP Server in Koha settings, ensuring that the server
    you create is valid and passes testing
c)  Run ./misc/export_records.pl with no agruments
    1)  Notice how koha.mrc file is generated
d)  Run ./misc/export_records.pl --destination_server_id=1
    1)  Notice how koha.mrc file is generated
    2)  On your FTP/SFTP Server, notice how koha.mrc is now present in the
        upload directory
e)  Run ./misc/export_records.pl --delete_local_after_run
    1)  Notice how koha.mrc file is NOT generated locally
f)  Run ./misc/export_records.pl --destination_server_id=1 --delete_local_after_run
    1)  Notice how koha.mrc file is NOT generated locally
    2)  On your FTP/SFTP Server, notice how koha.mrc is still present in the
        upload directory AND the file age is fresh

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 6 Martin Renvoize (ashimema) 2025-09-24 06:22:05 UTC
Created attachment 186856 [details] [review]
Bug 38115: (follow-up) Update terminology from SFTP/FTP to file transport

This follow-up patch updates the terminology in export_records.pl to use
generic 'file transport' language instead of specific 'FTP/SFTP' references,
aligning with the recent renaming of the underlying classes to File::Transports.

Changes:
- Replace $sftp_server variable with $file_transport for clarity
- Update error messages to use 'file transport server' terminology
- Fix incorrect error message for report_id validation
- Update POD documentation to reference 'file transport servers'
- Maintain consistency with the renamed admin interface and API

This ensures the script terminology matches the current class structure
and admin interface naming conventions.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 7 Martin Renvoize (ashimema) 2025-09-24 06:22:10 UTC
Created attachment 186857 [details] [review]
Bug 38115: (follow-up) Improve error handling and connection management

Enhance the export_records.pl script with better error handling
and connection lifecycle management:

1. Connection Management:
   - Always call disconnect() after file transport operations
   - Ensure connections are cleaned up even on errors

2. Error Handling:
   - Replace unusual die+pod2usage with clear error messages
   - Add file existence validation before upload attempts
   - Provide detailed error messages with context
   - Consistent error message formatting

3. User Experience:
   - Add validation for --delete_local_after_run without --destination_server_id
   - Provide informative success messages to STDERR
   - Better error messages with specific failure reasons

4. Robustness:
   - Check if output file was created before upload
   - Check if file exists before attempting deletion
   - Graceful handling of missing upload directories

These improvements make the script more reliable and user-friendly
while maintaining all existing functionality.