Skip to content

Core: Make build_api_url an instance method. - #8747

Merged
busunkim96 merged 1 commit into
googleapis:masterfrom
busunkim96:cloud-core-update
Jul 26, 2019
Merged

Core: Make build_api_url an instance method.#8747
busunkim96 merged 1 commit into
googleapis:masterfrom
busunkim96:cloud-core-update

Conversation

@busunkim96

Copy link
Copy Markdown
Contributor

See #8737.

Hmm, that @classmethod looks a little bogus to me. All actual uses of it are via instances:

$ git grep "\.build_api_url"
bigquery/tests/unit/test__http.py: self.assertEqual(conn.build_api_url("/foo"), URI)
bigquery/tests/unit/test__http.py: uri = conn.build_api_url("/foo", {"bar": "baz"})
bigquery/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")
bigquery/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")
core/google/cloud/_http.py: url = self.build_api_url(
core/tests/unit/test__http.py: self.assertEqual(conn.build_api_url("/foo"), URI)
core/tests/unit/test__http.py: uri = conn.build_api_url("/foo", {"bar": "baz", "qux": ["quux", "corge"]})
dns/tests/unit/test__http.py: self.assertEqual(conn.build_api_url("/foo"), URI)
dns/tests/unit/test__http.py: uri = conn.build_api_url("/foo", {"bar": "baz"})
dns/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")
logging/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")
resource_manager/tests/unit/test__http.py: self.assertEqual(conn.build_api_url("/foo"), URI)
resource_manager/tests/unit/test__http.py: uri = conn.build_api_url("/foo", {"bar": "baz"})
resource_manager/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")
runtimeconfig/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")
storage/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")
storage/tests/unit/test__http.py: self.assertEqual(conn.build_api_url("/foo"), URI)
storage/tests/unit/test__http.py: uri = conn.build_api_url("/foo", {"bar": "baz"})
translate/tests/unit/test__http.py: self.assertEqual(conn.build_api_url("/foo"), URI)
translate/tests/unit/test__http.py: uri = conn.build_api_url("/foo", query_params=query_params)
translate/tests/unit/test__http.py: expected_uri = conn.build_api_url("/rainbow")

@busunkim96
busunkim96 requested a review from tseaverJuly 24, 2019 20:55
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 24, 2019
@tseaver

Copy link
Copy Markdown
Contributor

@busunkim96 This is a case where we'd actually like to test the change against the affected API libraries (bigquery, dns, logging, resource_manager, runtime_config, storage, translate), but our heuristic for determining whether to run them can't detect that. Can you verify that they still pass with this change?

@busunkim96
busunkim96 requested a review from a teamJuly 24, 2019 22:28
@busunkim96

busunkim96 commented Jul 25, 2019

Copy link
Copy Markdown
ContributorAuthor

BigQuery unit test failures seem to be unrelated:

=================================== FAILURES ===================================
___________________ test_bq_to_arrow_array_w_special_floats ____________________
module_under_test = <module 'google.cloud.bigquery._pandas_helpers' from '/tmpfs/src/github/google-cloud-python/bigquery/google/cloud/bigquery/_pandas_helpers.py'>
@pytest.mark.skipif(pandas is None, reason="Requires `pandas`")
@pytest.mark.skipif(pyarrow is None, reason="Requires `pyarrow`")
def test_bq_to_arrow_array_w_special_floats(module_under_test):
bq_field = schema.SchemaField("field_name", "FLOAT64")
rows = [float("-inf"), float("nan"), float("inf"), None]
series = pandas.Series(rows, dtype="object")
arrow_array = module_under_test.bq_to_arrow_array(series, bq_field)
roundtrip = arrow_array.to_pylist()
assert len(rows) == len(roundtrip)
assert roundtrip[0] == float("-inf")
> assert roundtrip[1] != roundtrip[1] # NaN doesn't equal itself.
E assert None != None
tests/unit/test__pandas_helpers.py:491: AssertionError
=============================== warnings summary =============```

@tseaver

Copy link
Copy Markdown
Contributor

The BigQuery failure is likely due to a new version of a dependency (pyarrow would be my guess).

The Logging failure is a known flakiness in systest teardown, #8676.

@tseaver

Copy link
Copy Markdown
Contributor

@busunkim96 Do you want to back out d4f53bf from the PR before merging?

@tseaver

Copy link
Copy Markdown
Contributor

Also, a rebase would likely fix the BigQuery failure, if you'd like to do that first.

@busunkim96
busunkim96 merged commit f6d1f1d into googleapis:masterJul 26, 2019
@busunkim96
busunkim96 deleted the cloud-core-update branch July 26, 2019 17:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: corecla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@busunkim96@tseaver@googlebot