-
Notifications
You must be signed in to change notification settings - Fork 647
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
1117/s3 dl mtd future should fail when request fails #1122
1117/s3 dl mtd future should fail when request fails #1122
Conversation
val source = Source | ||
.fromFuture(future.flatMap(entityForSuccess).map(_._1)) |
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.
I don't see what's changing here. Aren't you just changing val future
to include flatMap(entityForSuccess)
, rather than doing it in-line in the source declaration?
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.
@jypma , this affects the error handling for the object metadata future as well, it will now fail on failed responses (i.e.404)
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.
Ahh I see. So the effect of the change is not here, but in the line 103 which maps future
which now has error handling because of the added .flatMap(entityForSuccess)
.
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.
exactly
Awesome. Thanks for the fix! |
* 1117: fix the issue by extracting errors before looking at the returned headers.
fix issue #1117,
the future returned by S3Stream.download should fail when the underlying request to aws s3 fails (i.e. with a 404).
this PR first modified an existing test to check the future (in addition to the source) for failure, then it introduces a fix.