IO: Fix S3 Error Handling #33645
IO: Fix S3 Error Handling #33645
Conversation
| @@ -62,7 +62,7 @@ def __init__( | |||
| # Extract compression mode as given, if dict | |||
| compression, self.compression_args = get_compression_method(compression) | |||
|
|
|||
| self.path_or_buf, _, _, _ = get_filepath_or_buffer( | |||
| self.path_or_buf, _, _, self.should_close = get_filepath_or_buffer( | |||
There was a problem hiding this comment.
@TomAugspurger - you pointer here was the exact problem #32486 (comment)
| @@ -92,7 +92,7 @@ def write( | |||
| **kwargs, | |||
| ): | |||
| self.validate_dataframe(df) | |||
| path, _, _, _ = get_filepath_or_buffer(path, mode="wb") | |||
| path, _, _, should_close = get_filepath_or_buffer(path, mode="wb") | |||
There was a problem hiding this comment.
For some reason we only have this logic in read. Should add to write too
There was a problem hiding this comment.
hmm, can you open an issue for this
There was a problem hiding this comment.
For which bit? File basically needs to be closed post write for s3fs to throw as expected. @TomAugspurger mention it here: #32486 (comment)
There was a problem hiding this comment.
For some reason we only have this logic in read. Should add to write too
your comment here.
|
Seems pretty reasonable to me if green. Can you add a whatsnew? |
| @@ -56,7 +56,15 @@ def open(*args): | |||
|
|
|||
| monkeypatch.setattr("gcsfs.GCSFileSystem", MockGCSFileSystem) | |||
| df1.to_csv("gs://test/test.csv", index=True) | |||
| df2 = read_csv(StringIO(s.getvalue()), parse_dates=["dt"], index_col=0) | |||
There was a problem hiding this comment.
This no longer works since file needs to be closed to validate credentials. #32486 (comment)
| @@ -86,28 +94,6 @@ def open(self, path, mode="r", *args): | |||
| ) | |||
|
|
|||
|
|
|||
| @td.skip_if_no("gcsfs") | |||
| def test_gcs_get_filepath_or_buffer(monkeypatch): | |||
There was a problem hiding this comment.
Removed since def test_to_csv_gcs now tests same functionality and uses identical monkeypatch.
| tips_df.to_csv("s3://an_s3_bucket_data_doesnt_exit/not_real.csv") | ||
|
|
||
| def test_write_s3_parquet_fails(self, tips_df): | ||
| # GH 27679 |
There was a problem hiding this comment.
Does this need an importorskip, or do we raise prior to importing the engine?
There was a problem hiding this comment.
Added an importskip. Would raise ImportError if PyArrow or fastparquet isn’t installed.
There was a problem hiding this comment.
can you add the decorator version instead
There was a problem hiding this comment.
have done - and changed other importerskip in this file
Co-Authored-By: Tom Augspurger <[email protected]>
|
Thanks for the review @TomAugspurger |
| @@ -92,7 +92,7 @@ def write( | |||
| **kwargs, | |||
| ): | |||
| self.validate_dataframe(df) | |||
| path, _, _, _ = get_filepath_or_buffer(path, mode="wb") | |||
| path, _, _, should_close = get_filepath_or_buffer(path, mode="wb") | |||
There was a problem hiding this comment.
hmm, can you open an issue for this
| tips_df.to_csv("s3://an_s3_bucket_data_doesnt_exit/not_real.csv") | ||
|
|
||
| def test_write_s3_parquet_fails(self, tips_df): | ||
| # GH 27679 |
There was a problem hiding this comment.
can you add the decorator version instead
|
green @jreback |
|
thanks @alimcmaster1 |
Co-authored-by: alimcmaster1 <[email protected]>
closes #27679
closes #32486
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff