IATI / ckanext-iati

CKAN extension for the IATI Registry
http://iatiregistry.org
9 stars 6 forks source link

CSV upload failure due to too many rows has hard-coded value #437

Open robredpath opened 8 months ago

robredpath commented 8 months ago

In the script that handles CSV file upload validation the error message has the value of 50 rows hard-coded, but the error is triggered based on exceeding a threshold in configuration.

It would be useful to see the real limit in the error message.

Side note: is there a reason this was set to 50 rows specifically? We have a publisher looking to publish hundreds of small files, which is obviously a different setup from most publishers but permitted by the standard. If it would cause any issues for this limit to be, say, 1000 - that would be useful to understand.

cormachallinanderilinx commented 8 months ago

Hi @robredpath I can quickly updated this to 1000 on staging, if you want to test it out?

We are not sure why it was set to 50 (or 101), other than the developer at the time was weary of expensive queries.

cormachallinanderilinx commented 8 months ago

Hi @robredpath Actually found this from a few years back, seems there was some issues and they decided to set a max upload: https://github.com/IATI/ckanext-iati/issues/286

robredpath commented 8 months ago

Thanks @cormachallinanderilinx - in the short term can we just make the message display the configured value, and we'll see what makes sense for the publisher whose data's led us to investigate this.

siwhitehouse commented 8 months ago

We have currently set the upload limit to 100 files and applied this to both Staging and Live instances.

https://github.com/IATI/ckanext-iati/pull/439 is part of that implementation.