Skip to content

Remove standalone uses of the apitools HttpError class #37296

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.

Sign up for GitHub

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

Open
jrmccluskey wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Remove standalone uses of the apitools HttpError class #37296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
145c0f4
Remove standalone uses of the apitools HttpError class
jrmccluskey Jan 13, 2026
05937ea
Update sdks/python/apache_beam/testing/pipeline_verifiers_test.py
jrmccluskey Jan 14, 2026
f9e7a2f
linting and formatting
jrmccluskey Jan 14, 2026
d9e894c
more linting and formatting
jrmccluskey Jan 14, 2026
e19cba0
even more linting
jrmccluskey Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions sdks/python/apache_beam/dataframe/io_it_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@
_LOGGER = logging.getLogger(__name__)

try:
from apitools.base.py.exceptions import HttpError
from google.api_core.exceptions import GoogleAPICallError
except ImportError:
HttpError = None
GoogleAPICallError = None


@unittest.skipIf(HttpError is None, 'GCP dependencies are not installed')
@unittest.skipIf(
GoogleAPICallError is None, 'GCP dependencies are not installed')
class ReadUsingReadGbqTests(unittest.TestCase):
@pytest.mark.it_postcommit
def test_ReadGbq(self):
Expand Down
7 changes: 4 additions & 3 deletions sdks/python/apache_beam/dataframe/io_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@
from apache_beam.testing.util import equal_to

try:
from apitools.base.py.exceptions import HttpError
from google.api_core.exceptions import GoogleAPICallError
except ImportError:
HttpError = None
GoogleAPICallError = None

# Get major, minor version
PD_VERSION = tuple(map(int, pd.__version__.split('.')[0:2]))
Expand Down Expand Up @@ -440,7 +440,8 @@ def test_double_write(self):
set(self.read_all_lines(output + 'out2.csv*')))


@unittest.skipIf(HttpError is None, 'GCP dependencies are not installed')
@unittest.skipIf(
GoogleAPICallError is None, 'GCP dependencies are not installed')
class ReadGbqTransformTests(unittest.TestCase):
@mock.patch.object(BigQueryWrapper, 'get_table')
def test_bad_schema_public_api_direct_read(self, get_table):
Expand Down
8 changes: 0 additions & 8 deletions sdks/python/apache_beam/io/gcp/datastore/v1new/datastoreio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
# Protect against environments where datastore library is not available.
# pylint: disable=wrong-import-order, wrong-import-position
try:
from apitools.base.py.exceptions import HttpError
from google.api_core.exceptions import ClientError
from google.api_core.exceptions import GoogleAPICallError
except ImportError:
Expand Down Expand Up @@ -309,9 +308,6 @@ def process(self, query, *unused_args, **unused_kwargs):
# e.code.value contains the numeric http status code.
service_call_metric.call(e.code.value)
raise
except HttpError as e:
service_call_metric.call(e)
raise
Copy link
Contributor

@damccorm damccorm Jan 14, 2026

Choose a reason for hiding this comment

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

Is this safe to remove because we're already not using apitools for these calls?

Copy link
Contributor

@damccorm damccorm Jan 14, 2026

Choose a reason for hiding this comment

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

Same question applies elsewhere - will this let actual HttpErrors through if we remove it?

Copy link
Contributor Author

@jrmccluskey jrmccluskey Jan 14, 2026

Choose a reason for hiding this comment

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

Yeah these clients shouldn't be returning these exception types anyway, the GoogleAPICallError should be used instead. We could add a broader exception catch here if we wanted to make absolutely sure though

Copy link
Contributor

@damccorm damccorm Jan 14, 2026

Choose a reason for hiding this comment

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

I think it is ok to leave as is if these clients shouldn't return the exception. We can't report the error code correctly with a broader catch anyways.



class _Mutate(PTransform):
Expand Down Expand Up @@ -469,10 +465,6 @@ def write_mutations(self, throttler, rpc_stats_callback, throttle_delay=1):
service_call_metric.call(e.code.value)
rpc_stats_callback(errors=1)
raise
except HttpError as e:
service_call_metric.call(e)
rpc_stats_callback(errors=1)
raise

def process(self, element):
client_element = self.element_to_client_batch_item(element)
Expand Down
11 changes: 0 additions & 11 deletions sdks/python/apache_beam/io/gcp/experimental/spannerio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@
# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports
# pylint: disable=unused-import
try:
from apitools.base.py.exceptions import HttpError
from google.api_core.exceptions import ClientError
from google.api_core.exceptions import GoogleAPICallError
from google.cloud.spanner import Client
Expand Down Expand Up @@ -437,9 +436,6 @@ def process(self, element, spanner_transaction):
except (ClientError, GoogleAPICallError) as e:
metric_action(metric_id, e.code.value)
raise
except HttpError as e:
metric_action(metric_id, e)
raise


@with_input_types(ReadOperation)
Expand Down Expand Up @@ -668,9 +664,6 @@ def process(self, element):
except (ClientError, GoogleAPICallError) as e:
self.service_metric(str(e.code.value))
raise
except HttpError as e:
self.service_metric(str(e))
raise

def teardown(self):
if self._snapshot:
Expand Down Expand Up @@ -1271,10 +1264,6 @@ def process(self, element):
for service_metric in self.service_metrics.values():
service_metric.call(str(e.code.value))
raise
except HttpError as e:
for service_metric in self.service_metrics.values():
service_metric.call(str(e))
raise
else:
for service_metric in self.service_metrics.values():
service_metric.call('ok')
Expand Down
6 changes: 3 additions & 3 deletions sdks/python/apache_beam/testing/pipeline_verifiers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@
]

try:
from apitools.base.py.exceptions import HttpError
from google.api_core.exceptions import GoogleAPICallError
except ImportError:
HttpError = None
GoogleAPICallError = None # type: ignore

MAX_RETRIES = 4

Expand Down Expand Up @@ -76,7 +76,7 @@ def describe_mismatch(self, pipeline_result, mismatch_description):
def retry_on_io_error_and_server_error(exception):
"""Filter allowing retries on file I/O errors and service error."""
return isinstance(exception, IOError) or \
(HttpError is not None and isinstance(exception, HttpError))
(GoogleAPICallError is not None and isinstance(exception, GoogleAPICallError)) # pylint: disable=line-too-long


class FileChecksumMatcher(BaseMatcher):
Expand Down
17 changes: 8 additions & 9 deletions sdks/python/apache_beam/testing/pipeline_verifiers_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@
try:
# pylint: disable=wrong-import-order, wrong-import-position
# pylint: disable=ungrouped-imports
from apitools.base.py.exceptions import HttpError
from google.api_core.exceptions import GoogleAPICallError
from google.api_core.exceptions import NotFound

from apache_beam.io.gcp.gcsfilesystem import GCSFileSystem
except ImportError:
HttpError = None
GoogleAPICallError = None # type: ignore
NotFound = None # type: ignore
GCSFileSystem = None # type: ignore


Expand Down Expand Up @@ -122,15 +124,12 @@ def test_file_checksum_matcher_read_failed(self, mock_match):
self.assertEqual(verifiers.MAX_RETRIES + 1, mock_match.call_count)

@patch.object(GCSFileSystem, 'match')
@unittest.skipIf(HttpError is None, 'google-apitools is not installed')
@unittest.skipIf(
GoogleAPICallError is None, 'GCP dependencies are not installed')
def test_file_checksum_matcher_service_error(self, mock_match):
mock_match.side_effect = HttpError(
response={'status': '404'},
url='',
content='Not Found',
)
mock_match.side_effect = NotFound('Not Found')
matcher = verifiers.FileChecksumMatcher('gs://dummy/path', Mock())
with self.assertRaises(HttpError):
with self.assertRaises(NotFound):
hc_assert_that(self._mock_result, matcher)
self.assertTrue(mock_match.called)
self.assertEqual(verifiers.MAX_RETRIES + 1, mock_match.call_count)
Expand Down
Loading