omeka-s-modules / FileSideload

GNU General Public License v3.0
2 stars 3 forks source link

Sideload Module Causes Lag in Item Edit / Create forms #35

Open pbcGIS opened 2 months ago

pbcGIS commented 2 months ago

I can't say whether this is an issue with the sideload module or with Omeka-S core 4.04. I'm cross posting this on the Omeka-S forum.

I noticed very long (2 minute) lag times in loadng the forms for Create Item or Edit Item. Long story short, Disabling the Sideload module (1.7.1) caused the lag to completely go away.

The reason that this emerged for me may be the following:

My sideload folder has nearly 30,000 files in it. My thheory is that for some reason, the code for the forms in question is trying to do something with all of these files.

Thanks

pbcGIS commented 2 weeks ago

PS It probably was not anticipated that Omeka administrators would end up with lots of files in the sideload directory, but it is convenient for us, since this is the cloud-location that we use for staging all of our imports. It also serves as an archive for the original images and the CSV Import manifests. This is very convenient. What is not convenient is having to remember to disable the side-load module after every import.

I'm trying to figure out what is causing this, because I'd like to fix it. Methinks that this module extends an Omeka-S class in such a way that when this class runs in the whatever function opens an item for editing, it has to scan all of the files in the sideload directory (maybe looking for locks?) In any case, there should be no reason for the number of files in the sideload directory should ne an issue when opening an item for editing.

zerocrates commented 2 weeks ago

Not the initial direct developer of this module but I'd say you're right on that the design doesn't anticipate users with 30,000 files in the sideload folder. I'm not 100% sure without further investigation exactly where the bottleneck lies: simply having a select with 30,000 options is going to cause a huge page that's slow to send and slow to render even without considering the time spent listing the directory and checking all the files.

Let's start here: https://github.com/omeka-s-modules/FileSideload/blob/master/src/Media/Ingester/Sideload.php#L132

If you replace this line with

$files = [];

you'll be skipping the listing of files and their inclusion in the sideload media form.

pbcGIS commented 2 weeks ago

That makes sense. I had overlooked the function that suggests files in the sideload folder. Your suggestion pointed me in the right direction. Changing that one line did not solve the problem. What did was going to config/module.config.php and commenting out the entire section that registers the sideload ingester with the general media_ingesters.

>      6  return [
>      7  //    'media_ingesters' => [
>      8  //        'factories' => [
>      9  //            'sideload' => Service\MediaIngesterSideloadFactory::class,
>     10  //            'sideload_dir' => Service\MediaIngesterSideloadDirFactory::class,
>     11  //        ],
>     12  //    ],
>     13      'translator' => [
> 

This may not be a hack that most people want to do if they are going to use sideload as a potential media source for ad-hoc item creation. We have not thought that far ahead. For now all of our imports are through a fork of the CSVImport module.

Maybe after we get into the phase of ad-hoc item creation, we will eventually develop a workflow that does not involve leaving our initial import exchange packages in the sideload directory -- although that is very convenient.

Just thinking out loud I wonder if presenting the sideload files as a directory tree -- or auto-complete?-- rather than a fully enumerated list of files might be a way to have it all. I recognize that these things are not easy to create.

In any case, this solution will help us out in the short term. Thank you John!

zerocrates commented 2 weeks ago

Hmm, I'm somewhat surprised my initial suggestion didn't work, as that's the part of the module's code that actually reads the files from the sideload folder.

It's possible you might have a similar issue from the "sideload dir" ingester; the equivalent line you could just comment out to make it not read from the sideload folder would be here https://github.com/omeka-s-modules/FileSideload/blob/master/src/Media/Ingester/SideloadDir.php#L176

I'd be somewhat concerned that your removal of the sideload ingester registration in its entirety would also break your future imports; in general, those use the ingesters, so removing the reigstration could be a problem.

I agree that any of these things we've talked about here are more of hacks. I'm not certain on what, if anything, could be done here that would work in a more generalized way. There are some possible options like putting a cap on the number of options that would be included in the UI, or changing the UI completely.

pbcGIS commented 2 weeks ago

Right, and Right! My hatchet-job on the module.config.php did break the CSV import tool of media. Commenting out the initialization of the directory listing as you suggested prevents the unwanted overload on the Item Edit page and preserves the proper function of the CSV Import tool.

Thank you VERY much!

On Tue, Aug 20, 2024 at 2:43 PM John Flatness @.***> wrote:

Hmm, I'm somewhat surprised my initial suggestion didn't work, as that's the part of the module's code that actually reads the files from the sideload folder.

It's possible you might have a similar issue from the "sideload dir" ingester; the equivalent line you could just comment out to make it not read from the sideload folder would be here https://github.com/omeka-s-modules/FileSideload/blob/master/src/Media/Ingester/SideloadDir.php#L176

I'd be somewhat concerned that your removal of the sideload ingester registration in its entirety would also break your future imports; in general, those use the ingesters, so removing the reigstration could be a problem.

I agree that any of these things we've talked about here are more of hacks. I'm not certain on what, if anything, could be done here that would work in a more generalized way. There are some possible options like putting a cap on the number of options that would be included in the UI, or changing the UI completely.

— Reply to this email directly, view it on GitHub https://github.com/omeka-s-modules/FileSideload/issues/35#issuecomment-2299514439, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIFNNTZD2MYYQ47RCL3TZDZSOE5DAVCNFSM6AAAAABKHWQWV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJZGUYTINBTHE . You are receiving this because you authored the thread.Message ID: @.***>

-- Paul B. Cote Geographic Information Services cultivating spatial intelligence [tm] http://www.pbcgis.com pbcGIS.com