athrog commented on pull request #120: URL: https://github.com/apache/solr/pull/120#issuecomment-895462948
Thanks again @HoustonPutman for your time and energy on this PR. > This is a major change, but I think a big improvement in terms of usability. Is there a reason why you mandated paths that start with / in the first place? This doesn't seem to be an S3 standard, and is definitely not easy to do when creating directories in the S3 UI... @psalagnac might know better than me, but looking through our internal commit history, I don't see any clear reason why we chose to require a leading slash. Given our logic for parsing out parent dirs, maybe there was some code in Solr 7 (the version we were on at the time) that choked on it not starting with a slash? I agree with your logic, and no time like the present to fix it. Your other fixes look good to me -- I like using content-type as an indicator for dirs, and the `createDirectoryUri` refactor is also nice. Have you been testing against real s3? Or against the mock s3 locally? After syncing the latest, I'm no longer able to restore with mock s3. Backups work fine (I checked the files on disk), but I get `Couldn't restore since doesn't exist: s3://backup1628535176/collection1628535176` error messages when hitting `solr/admin/collections?action=RESTORE&repository=s3&location=s3:&name=$BACKUP_NAME&collection=$RESTORED_COLLECTION_NAME` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
