-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix host used in minio URLs #1487
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1487 +/- ##
==========================================
+ Coverage 88.54% 88.62% +0.07%
==========================================
Files 77 77
Lines 10565 10565
==========================================
+ Hits 9355 9363 +8
+ Misses 1210 1202 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -29,7 +29,7 @@ services: | |||
DJANGO_MINIO_STORAGE_ENDPOINT: minio:9000 | |||
DJANGO_MINIO_STORAGE_SECRET_KEY: minioSecretKey | |||
DJANGO_STORAGE_BUCKET_NAME: django-storage | |||
DJANGO_MINIO_STORAGE_MEDIA_URL: http://localhost:9000/django-storage | |||
DJANGO_MINIO_STORAGE_MEDIA_URL: http://127.0.0.1:9000/django-storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is odd that it would need it for MINIO but nothing else... but ok, if that resolves it for you I do not mind, but please add a brief comment above on why it is needed. ATM not even commit message provides an explanation -- would be hard to figure out why it was needed and thus might get reverted etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment added: 85f6dd8
🚀 PR was released in |
Using
localhost
was causing test failures for me locally when fsspec was involved, likely due tolocalhost
also resolving to::1
, which the minio port was not exposed on.