IRSA-6468: Support for Packager Script of URLs (Euclid, SphereX, Rubin, etc.) #1690

kpuriIpac · 2025-01-11T03:57:24Z

IFE PR
Based on feedback, created a DownloadSciptWorker based on our existing PackagingWorker with changes to write out the URLs to a script made available for download (instead of zipped files of products)
Client can specify the script type (curl, wget or plain list of urls)
Removed cutout option from the UI (a new ticket will address this item).
Modified DownloadScript.java to support better filename generation from the given URL
Made some changes in the DownloadButton and DownloadOptionPanel components to accept the download button text (instead of Prepare Download)
DownloadOptionPanel and PackageRequest (in SearchServicesJson.js) now accept a new parameter: downloadType. This determines whether we use our traditional packager or the new download script.
Got rid of downloading data to a tmp file in ObsCorePackagaer.java

Testing:

Euclid: https://irsa-6468-packager-script.irsakudev.ipac.caltech.edu/applications/euclid
Firefly: https://fireflydev.ipac.caltech.edu/irsa-6468-packager-script/firefly
DCE: https://irsa-6468-packager-script.irsakudev.ipac.caltech.edu/irsaviewer/dce
For Euclid, do an Image search (previously Explore Regions)
- select any number of rows, and click on Generate Download Script
- test downloading all 3 script types (curl, wget, and plain urls)
- try and choose different data products, including All data
- I would suggest even running one of the downloaded scripts (when you select curl or wget) to ensure the download works (you can only keep 2-3 URLs in the file to test this)
In Firefly, do a CADC TAP Search
- Under tables, select IVOA ObsCore, you may search for m81 or anything else
- Select 1 or more rows and click on Prepare Download, ensure this behavior is the same as before as ObsCorePackaager.java has also been modified

loitly · 2025-01-13T18:03:58Z

src/firefly/java/edu/caltech/ipac/firefly/server/query/PackagerScript.java

+import static edu.caltech.ipac.util.FileUtil.getUniqueFileNameForGroup;
+
+@SearchProcessorImpl(id = "PackagerScript")
+public class PackagerScript extends FileGroupsProcessor {


This class implementation is not right. If it's a generic, reusable script generator, then it should not have any ObsCore logic. If it's an ObsCore data packager, then it is named wrong and there is already an ObsCorePackager.

loitly

The solution should be divided into two parts:

Verify that the script generator functions as intended.
Modify the UI and server code to support a "create download script" option as an alternative to the existing "package download" data feature.

I thought that this PR was suppose to address the first part. In any case, the implementation is not what I expected.

kpuriIpac · 2025-01-15T04:54:54Z

src/firefly/java/edu/caltech/ipac/firefly/server/query/ObsCorePackager.java

                File tmpFile = File.createTempFile("obscorepackager-", ".download", ServerContext.getTempWorkDir());
                FileInfo tmpFileInfo = URLDownload.getDataToFile(url, tmpFile);
                String contentType = tmpFileInfo.getAttribute(HttpResultInfo.CONTENT_TYPE);

+                //todo: replace tmpFile code above with this, or else remove this commented out code
+                //HttpResultInfo r= URLDownload.getDataFromURL(new URL(access_url), null, null, null);
+                //String contentType = r.getContentType();


FYI @robyww @loitly

based on our discussion, I had thought about removing the tmpFile code, as we could just use access_format to get the extension, but I realized that the else case (~line 103) is actually triggered when access_format is null. So I have left this code and the commented out line here for now, but we can discuss it.

kpuriIpac · 2025-01-16T03:18:39Z

@robyww @loitly:

Since the PR description is a bit detailed, here's a short summary of today's final changes:

DownloadOptionPanel and PackageRequest (in SearchServicesJson.js) now accept a new parameter: downloadType. This determines whether we use our traditional packager or the new download script.
Got rid of downloading data to a tmp file in ObsCorePackagaer.java
cutout has been removed from the UI as an option in Euclid

Follow the testing instructions above and in the IFE PR, but a quick summary for testing as well:

test doing an Image search in Euclid, and download all 3 script types (curl, wget, plain urls)
test doing an Inspect Objects and Search by ID search in Euclid as well (this is just to ensure Euclid is ready for our internal release)
Test doing a CADC ivoa ObsCore search in firefly tap, then select a few rows and click Prepare Download. Ensure this behaves the same as before (you should get a zipped file with the data products).

loitly

Looks good. Minor cleanup then you're good to go.

loitly · 2025-01-16T18:02:55Z

src/firefly/java/edu/caltech/ipac/firefly/server/packagedata/DownloadScriptWorker.java

+    public static final String PRODUCTS = "productTypes";
+    public static final String SCRIPT = "scriptType";
+    public static final String ACCESS_URL = "access_url";
+    public static final String ACCESS_FORMAT = "access_format";
+    public static final String SEMANTICS = "semantics";


These constants should not be here.

loitly · 2025-01-16T18:37:41Z

src/firefly/java/edu/caltech/ipac/firefly/server/packagedata/DownloadScriptWorker.java

+    private static String makeDownloadUrl(File f, String suggestedName) {
+        String fileStr = ServerContext.replaceWithPrefix(f);


No need to copy/paste this function here. Change PackagingWorker.makeDownloadUrl accessor to public or package protected(remove private) then use it instead.

loitly · 2025-01-16T18:52:24Z

src/firefly/js/ui/DownloadDialog.jsx

+                                         showFileLocation=true, showTitle=true,
+                                         downloadButtonText='Prepare Download', downloadType='package', ...props}) {


We could derive downloadButtonText and similarly buttonText from downloadType to avoid redundancy. However, I'm open to whichever approach you prefer.

did this for downloadButtonText based on your suggestion, left buttonText as is, since it's passed to the DownloadButton component (as opposed to DownloadOptionPanel)

kpuriIpac · 2025-01-16T19:31:39Z

src/firefly/java/edu/caltech/ipac/firefly/server/query/ObsCorePackager.java

@@ -155,15 +156,12 @@ public static List<FileInfo> parseDatalink(URL url, String prepend_file_name, St
                        ext_file_name = makeDataLinkFolder ? prepend_file_name + "/" + ext_file_name : ext_file_name;
                    }

-                    if (products != null) {
-                        // Check if products exist and contains sem
+                    if (products != null) { // Check if products exist and contains sem


@robyww @loitly I left this as a null check, because I add a check for null & empty for the string productTypes at the beginning on computeFileGroup function above.

robyww · 2025-01-13T18:08:08Z

src/firefly/java/edu/caltech/ipac/firefly/server/util/DownloadScript.java

+
+        // Extract the last part of the path
+        String path = url.getPath();
+        if (path != null && !path.isEmpty()) {


use StringUtils.isEmpty

kpuriIpac added the enhancement label Jan 11, 2025

kpuriIpac added this to the 2025.1 milestone Jan 11, 2025

kpuriIpac requested a review from robyww January 11, 2025 03:57

loitly reviewed Jan 13, 2025

View reviewed changes

robyww modified the milestones: 2025.1, 2025.2 Jan 13, 2025

kpuriIpac force-pushed the IRSA-6468-packager-script branch 4 times, most recently from 5bf920a to b499a06 Compare January 15, 2025 04:24

kpuriIpac marked this pull request as ready for review January 15, 2025 04:37

kpuriIpac commented Jan 15, 2025

View reviewed changes

kpuriIpac force-pushed the IRSA-6468-packager-script branch from b499a06 to 80b64e8 Compare January 16, 2025 00:41

loitly approved these changes Jan 16, 2025

View reviewed changes

kpuriIpac force-pushed the IRSA-6468-packager-script branch from d246852 to 949d4fe Compare January 16, 2025 19:25

kpuriIpac commented Jan 16, 2025

View reviewed changes

robyww approved these changes Jan 16, 2025

View reviewed changes

IRSA-6468: Firefly support for a download script

a831bbc

kpuriIpac force-pushed the IRSA-6468-packager-script branch from 26c985b to a831bbc Compare January 16, 2025 22:56

kpuriIpac merged commit 16da88e into dev Jan 16, 2025

kpuriIpac deleted the IRSA-6468-packager-script branch January 16, 2025 22:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IRSA-6468: Support for Packager Script of URLs (Euclid, SphereX, Rubin, etc.) #1690

IRSA-6468: Support for Packager Script of URLs (Euclid, SphereX, Rubin, etc.) #1690

kpuriIpac commented Jan 11, 2025 •

edited

Loading

loitly Jan 13, 2025

loitly left a comment

kpuriIpac Jan 15, 2025

kpuriIpac commented Jan 16, 2025

loitly left a comment

loitly Jan 16, 2025

loitly Jan 16, 2025

loitly Jan 16, 2025

kpuriIpac Jan 16, 2025

kpuriIpac Jan 16, 2025

robyww Jan 13, 2025

		private static String makeDownloadUrl(File f, String suggestedName) {
		String fileStr = ServerContext.replaceWithPrefix(f);

		showFileLocation=true, showTitle=true,
		downloadButtonText='Prepare Download', downloadType='package', ...props}) {

IRSA-6468: Support for Packager Script of URLs (Euclid, SphereX, Rubin, etc.) #1690

IRSA-6468: Support for Packager Script of URLs (Euclid, SphereX, Rubin, etc.) #1690

Conversation

kpuriIpac commented Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

loitly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpuriIpac commented Jan 16, 2025

loitly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpuriIpac commented Jan 11, 2025 •

edited

Loading