Skip to content
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

[BEAM-522] Fixes GcsIO.exists() to properly handle files that do not exist #779

Merged
merged 2 commits into from
Aug 10, 2016

Conversation

chamikaramj
Copy link
Contributor

Currently this invocation fails for non existing files instead of returning false.

Updates FileSink.finalize_write() so that we capture and log any transient errors that get thrown at the channel_factory.exists() call.

@chamikaramj
Copy link
Contributor Author

R: @charlesccychen

@charlesccychen
Copy link
Contributor

Thanks! LGTM.

@chamikaramj
Copy link
Contributor Author

Thanks Charles.

R: @dhalperi

@chamikaramj
Copy link
Contributor Author

R: @kennknowles since there seems to be a lot of PR reviews on Dan's plate :)

try:
exists = channel_factory.exists(final_name)
except Exception as exists_e: # pylint: disable=broad-except
logging.warning('Exception when invoking channel_factory.exists(): '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message isn't likely to be actionable without knowing more about the factory or the names involved. Can you add any detail without leaking information?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the error message.

Currently this invocation fails for non existing files instead of returning false.

Updates FileSink.finalize_write() so that we capture and log any transient errors that get thrown at the channel_factory.exists() call.
@chamikaramj chamikaramj force-pushed the sink_finalize_fix_idempotency branch from 792c3b5 to 04c650f Compare August 8, 2016 23:36
@chamikaramj
Copy link
Contributor Author

Thanks Ken. PTAL.

@chamikaramj
Copy link
Contributor Author

Friendly ping :)

@kennknowles
Copy link
Member

LGTM

@asfgit asfgit merged commit 04c650f into apache:python-sdk Aug 10, 2016
asfgit pushed a commit that referenced this pull request Aug 10, 2016
@chamikaramj
Copy link
Contributor Author

Thanks Ken.

pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
* chore: Update gapic-generator-python to v1.11.7

PiperOrigin-RevId: 573230664

Source-Link: googleapis/googleapis@93beed3

Source-Link: https://github.com/googleapis/googleapis-gen/commit/f4a4edaa8057639fcf6adf9179872280d1a8f651
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZjRhNGVkYWE4MDU3NjM5ZmNmNmFkZjkxNzk4NzIyODBkMWE4ZjY1MSJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: Update gapic-generator-python to v1.11.8

PiperOrigin-RevId: 574178735

Source-Link: googleapis/googleapis@7307199

Source-Link: https://github.com/googleapis/googleapis-gen/commit/ce3af21b7c559a87c2befc076be0e3aeda3a26f0
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiY2UzYWYyMWI3YzU1OWE4N2MyYmVmYzA3NmJlMGUzYWVkYTNhMjZmMCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: Update gapic-generator-python to v1.11.9

PiperOrigin-RevId: 574520922

Source-Link: googleapis/googleapis@5183984

Source-Link: https://github.com/googleapis/googleapis-gen/commit/a59af19d4ac6509faedf1cc39029141b6a5b8968
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYTU5YWYxOWQ0YWM2NTA5ZmFlZGYxY2MzOTAyOTE0MWI2YTViODk2OCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Daniel Sanche <sanche@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants