Skip to content
This repository was archived by the owner on Mar 13, 2022. It is now read-only.

Prevent 503s from killing the client during discovery #187

Merged
merged 1 commit into from
Sep 15, 2020
Merged

Prevent 503s from killing the client during discovery #187

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Prevent 503s from killing the client during discovery
  • Loading branch information
@fabianvf
fabianvf committed Mar 13, 2020
commit fb259e1477997c8b16b8f8a09c410e7a506fd39c
17 changes: 9 additions & 8 deletions dynamic/discovery.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from urllib3.exceptions import ProtocolError, MaxRetryError

from kubernetes import __version__
from .exceptions import NotFoundError, ResourceNotFoundError, ResourceNotUniqueError, ApiException
from .exceptions import NotFoundError, ResourceNotFoundError, ResourceNotUniqueError, ApiException, ServiceUnavailableError
from .resource import Resource, ResourceList


Expand Down Expand Up @@ -155,7 +155,10 @@ def get_resources_for_api_version(self, prefix, group, version, preferred):
subresources = {}

path = '/'.join(filter(None, [prefix, group, version]))
resources_response = self.client.request('GET', path).resources or []
try:
resources_response = self.client.request('GET', path).resources or []
except ServiceUnavailableError:
resources_response = []
Comment on lines +158 to +161
Copy link
Member

Choose a reason for hiding this comment

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

I think when the exception is caught we should probably log a warning. What do you think?

logger.warn('some helpful text')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense, though this module is lacking logging in general. I can add it here and then maybe do a followup PR to add it to rest?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. 👍


resources_raw = list(filter(lambda resource: '/' not in resource['name'], resources_response))
subresources_raw = list(filter(lambda resource: '/' in resource['name'], resources_response))
Expand Down Expand Up @@ -251,13 +254,11 @@ def __search(self, parts, resources, reqParams):
# Check if we've requested resources for this group
if not resourcePart.resources:
prefix, group, version = reqParams[0], reqParams[1], part
try:
resourcePart.resources = self.get_resources_for_api_version(prefix,
group, part, resourcePart.preferred)
except NotFoundError:
raise ResourceNotFoundError
resourcePart.resources = self.get_resources_for_api_version(
prefix, group, part, resourcePart.preferred)

self._cache['resources'][prefix][group][version] = resourcePart
self.__update_cache=True
self.__update_cache = True
return self.__search(parts[1:], resourcePart.resources, reqParams)
elif isinstance(resourcePart, dict):
# In this case parts [0] will be a specified prefix, group, version
Expand Down