HoustonPutman commented on pull request #120: URL: https://github.com/apache/solr/pull/120#issuecomment-893609933
Sorry was actually going to comment today. I have some local changes that generally make the experience better, in my opinion. Will probably push soon, but the biggest change is this: It removes the ability for the s3 object name to start with a `/`. This is because figuring out the correct location URI to use was beyond confusing to me when the `/` was required to start. If we auto-remove any `/` at the begging of the location (and update some of the logic around this), then it's practically impossible for users to pass a `location` that results in an error. 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... I've made some other changes that I think are good and do not negatively impact s3 performance. Will post this patch when y'all have signed off on the change above ^. In the meantime there are things that need to be done before this is ready to be merged: - [ ] Documentation of the S3 Repo option in the solr-ref-guide - [ ] The tests currently have a lot of asserts without comments, these should be easy to debug when tests fail. Please add messages to any asserts there. - [ ] Finish debugging the async error, if it still occurs. -- 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
