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.

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

Jump to bottom

feat: expose reservation usage stats on jobs #524

Merged
merged 3 commits into from Feb 17, 2021
Merged

feat: expose reservation usage stats on jobs #524

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad85224
feat: expose reservation usage stats on jobs
plamut Feb 16, 2021
eeda4ac
Add ReservationUsage to job types in docs
plamut Feb 17, 2021
2e770de
Remove redundant space in docstring.
plamut Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions google/cloud/bigquery/job/__init__.py
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@
from google.cloud.bigquery.job.base import _DONE_STATE
from google.cloud.bigquery.job.base import _JobConfig
from google.cloud.bigquery.job.base import _JobReference
from google.cloud.bigquery.job.base import ReservationUsage
from google.cloud.bigquery.job.base import ScriptStatistics
from google.cloud.bigquery.job.base import ScriptStackFrame
from google.cloud.bigquery.job.base import UnknownJob
Expand Down Expand Up @@ -51,6 +52,7 @@
"_DONE_STATE",
"_JobConfig",
"_JobReference",
"ReservationUsage",
"ScriptStatistics",
"ScriptStackFrame",
"UnknownJob",
Expand Down
27 changes: 27 additions & 0 deletions google/cloud/bigquery/job/base.py
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@

"""Base classes and helpers for job classes."""

from collections import namedtuple
import copy
import http
import threading
Expand Down Expand Up @@ -73,6 +74,16 @@ def _error_result_to_exception(error_result):
)


ReservationUsage = namedtuple("ReservationUsage", "name slot_ms")
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting!

This is a little different from our usual "class with _properties dictionary" approach, but does seem to save us some boilerplate. I'll allow it. :-)

Do you think we should do this for our other read-only classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I though a namedtuple would better communicate that it's a read-only (output-only) property, although our "_properties classes" can also have read-only properties - but yeah, a namedtuple requires much less boilerplate.

(FWIW, I personally view these output-only classes merely as named read-only containers with descriptive attribute names, and using a class with a bunch of read-only properties is just an implementation detail)

We can eventually refactor other output-only classes to namedtuples for consistency, yes, but I wouldn't do it just yet. There's not that much immediate value in it, as that code has already been written, and there might be some legit use cases out there that we don't know about... let's first wait and see how a namedtuple performs when released into the wild.

ReservationUsage.__doc__ = " Job resource usage for a reservation."
plamut marked this conversation as resolved.
Show resolved Hide resolved
ReservationUsage.name.__doc__ = (
'Reservation name or "unreserved" for on-demand resources usage.'
)
ReservationUsage.slot_ms.__doc__ = (
"Total slot milliseconds used by the reservation for a particular job."
)


class _JobReference(object):
"""A reference to a job.

Expand Down Expand Up @@ -305,6 +316,22 @@ def _job_statistics(self):
statistics = self._properties.get("statistics", {})
return statistics.get(self._JOB_TYPE, {})

@property
def reservation_usage(self):
"""Job resource usage breakdown by reservation.

Returns:
List[google.cloud.bigquery.job.ReservationUsage]:
Reservation usage stats. Can be empty if not set from the server.
"""
usage_stats_raw = _helpers._get_sub_prop(
self._properties, ["statistics", "reservationUsage"], default=()
)
return [
ReservationUsage(name=usage["name"], slot_ms=int(usage["slotMs"]))
for usage in usage_stats_raw
]

@property
def error_result(self):
"""Error information about the job as a whole.
Expand Down
24 changes: 24 additions & 0 deletions tests/unit/job/test_base.py
View file
Open in desktop
Expand Up @@ -319,6 +319,30 @@ def test_ended(self):
stats["endTime"] = millis
self.assertEqual(job.ended, now)

def test_reservation_usage_no_stats(self):
client = _make_client(project=self.PROJECT)
job = self._make_one(self.JOB_ID, client)
job._properties["statistics"] = {}
self.assertEqual(job.reservation_usage, [])

def test_reservation_usage_stats_exist(self):
from google.cloud.bigquery.job import ReservationUsage

client = _make_client(project=self.PROJECT)
job = self._make_one(self.JOB_ID, client)
job._properties["statistics"] = {
"reservationUsage": [
{"name": "slot_foo", "slotMs": "42"},
{"name": "slot_bar", "slotMs": "123"},
],
}

expected = [
ReservationUsage(name="slot_foo", slot_ms=42),
ReservationUsage(name="slot_bar", slot_ms=123),
]
self.assertEqual(job.reservation_usage, expected)

def test__job_statistics(self):
statistics = {"foo": "bar"}
client = _make_client(project=self.PROJECT)
Expand Down