Refactor validations retrieval for performance

The goal of this commit is to reduce the average time spent retrieving
validations from Deckhand. Currently, wait times when committing
configdocs can be significant due to unnecessary API calls. This change
reduces the number of API calls during this process by utilizing the
`/revisions/{{revision_id}}/validations/detail` endpoint exposed by
Deckhand. During testing, this introduced a 71% decrease in cumulative
time for committing configdocs. Note, this commit does not introduce
usage of the official Deckhand client, which will be addressed in a
future change.

Change-Id: I3c86fca6bae1a5a2f74963a87b2198c1705cf3a6
This commit is contained in:
Drew Walters 2018-10-04 15:24:51 +00:00
parent e11c621a84
commit 6f8b9f858a
2 changed files with 55 additions and 156 deletions

View File

@ -41,11 +41,7 @@ class DeckhandPaths(enum.Enum):
REVISION_LIST = '/revisions'
REVISION_TAG_LIST = '/revisions/{}/tags'
REVISION_TAG = '/revisions/{}/tags/{}'
REVISION_VALIDATION_LIST = '/revisions/{}/validations'
REVISION_VALIDATION = '/revisions/{}/validations/{}'
REVISION_VALIDATION_ENTRY = (
'/revisions/{}/validations/{}/entries/{}'
)
REVISION_VALIDATIONS_LIST = '/revisions/{}/validations/detail'
ROLLBACK = '/rollback/{}'
@ -246,98 +242,26 @@ class DeckhandClient(object):
self._handle_bad_response(response)
return response.text
@staticmethod
def _build_validation_base(dh_validation):
# creates the base structure for validation response
return {
'count': dh_validation.get('count'),
'results': []
}
@staticmethod
def _build_validation_entry(dh_val_entry):
# creates a validation entry by stripping off the URL
# that the end user can't use anyway.
dh_val_entry.pop('url', None)
return dh_val_entry
def get_all_revision_validations(self, revision_id):
"""
Collects and returns the yamls of the validations for a
revision
"""
val_resp = {}
response = self._get_base_validation_resp(revision_id)
# if the base call is no good, stop.
self._handle_bad_response(response)
all_validation_resp = yaml.safe_load(response.text)
if all_validation_resp:
val_resp = DeckhandClient._build_validation_base(
all_validation_resp
)
validations = all_validation_resp.get('results')
for validation_subset in validations:
subset_name = validation_subset.get('name')
subset_response = self._get_subset_validation_response(
revision_id,
subset_name
)
# don't fail hard on a single subset not replying
# TODO (bryan-strassner) maybe this should fail hard?
# what if they all fail?
if subset_response.status_code >= 400:
LOG.error(
'Failed to retrieve %s validations for revision %s',
subset_name, revision_id
)
val_subset = yaml.safe_load(subset_response.text)
entries = val_subset.get('results')
for entry in entries:
entry_id = entry.get('id')
e_resp = self._get_entry_validation_response(revision_id,
subset_name,
entry_id)
if e_resp.status_code >= 400:
# don't fail hard on a single entry not working
# TODO (bryan-strassner) maybe this should fail hard?
# what if they all fail?
LOG.error(
'Failed to retrieve entry %s for category %s '
'for revision %s',
entry_id, subset_name, revision_id
)
entry = yaml.safe_load(e_resp.text)
val_resp.get('results').append(
DeckhandClient._build_validation_entry(entry)
)
return val_resp
Collects a YAML document containing a list of validation results
corresponding to a Deckhand revision ID.
def _get_base_validation_resp(self, revision_id):
# wraps getting the base validation response
:param revision_id: A Deckhand revision ID corresponding to a set of
documents.
:returns: A YAML document containing a results key mapped to a list of
validation results.
"""
# TODO(@drewwalters96): This method should be replaced with usage of
# the official Deckhand client.
url = DeckhandClient.get_path(
DeckhandPaths.REVISION_VALIDATION_LIST
DeckhandPaths.REVISION_VALIDATIONS_LIST
).format(revision_id)
return self._get_request(url)
response = self._get_request(url)
self._handle_bad_response(response)
def _get_subset_validation_response(self, revision_id, subset_name):
# wraps getting the subset level of detail of validations
subset_url = DeckhandClient.get_path(
DeckhandPaths.REVISION_VALIDATION
).format(revision_id, subset_name)
return self._get_request(subset_url)
def _get_entry_validation_response(self,
revision_id,
subset_name,
entry_id):
# wraps getting the entry level detail of validation
e_url = DeckhandClient.get_path(
DeckhandPaths.REVISION_VALIDATION_ENTRY
).format(revision_id, subset_name, entry_id)
return self._get_request(e_url)
return yaml.safe_load(response.text)
@staticmethod
def _handle_bad_response(response, threshold=400):

View File

@ -734,24 +734,7 @@ def test_generate_dh_val_message():
assert generated == expected
FK_VAL_BASE_RESP = FakeResponse(
status_code=200,
text="""
---
count: 2
next: null
prev: null
results:
- name: deckhand-schema-validation
url: https://deckhand/a/url/too/long/for/pep8
status: success
- name: promenade-site-validation
url: https://deckhand/a/url/too/long/for/pep8
status: failure
...
""")
FK_VAL_SUBSET_RESP = FakeResponse(
FK_VAL_RESP = FakeResponse(
status_code=200,
text="""
---
@ -759,77 +742,69 @@ count: 1
next: null
prev: null
results:
- id: 0
url: https://deckhand/a/url/too/long/for/pep8
- name: promenade-site-validation
url: https://deckhand/api/v1.0/revisions/4/etc
status: failure
...
""")
createdAt: 2017-07-16T02:03Z
expiresAfter: null
expiresAt: null
errors:
- documents:
- schema: promenade/Node/v1
name: node-document-name
- schema: promenade/Masters/v1
name: kubernetes-masters
message: This is a message.
- documents:
- schema: promenade/Node/v1
name: node-document-name
- schema: promenade/Masters/v1
name: kubernetes-masters
message: This is a message.
FK_VAL_ENTRY_RESP = FakeResponse(
status_code=200,
text="""
---
name: promenade-site-validation
url: https://deckhand/a/url/too/long/for/pep8
status: failure
createdAt: 2017-07-16T02:03Z
expiresAfter: null
expiresAt: null
errors:
- documents:
- schema: promenade/Node/v1
name: node-document-name
- schema: promenade/Masters/v1
name: kubernetes-masters
message: Node has master role, but not included in cluster masters list.
...
""")
def test__get_deckhand_validation_errors():
@mock.patch.object(DeckhandClient, 'get_all_revision_validations',
return_value=yaml.safe_load(FK_VAL_RESP.text))
def test__get_deckhand_validation_errors(mock_client):
"""
Tets the functionality of processing a response from deckhand
"""
helper = ConfigdocsHelper(CTX)
helper.deckhand._get_base_validation_resp = (
lambda revision_id: FK_VAL_BASE_RESP)
helper.deckhand._get_subset_validation_response = (
lambda reivsion_id, subset_name: FK_VAL_SUBSET_RESP)
helper.deckhand._get_entry_validation_response = (
lambda reivsion_id, subset_name, entry_id: FK_VAL_ENTRY_RESP)
assert len(helper._get_deckhand_validation_errors(5)) == 2
FK_VAL_ENTRY_RESP_EMPTY = FakeResponse(
FK_VAL_RESP_EMPTY = FakeResponse(
status_code=200,
text="""
---
name: promenade-site-validation
url: https://deckhand/a/url/too/long/for/pep8
status: failure
createdAt: 2017-07-16T02:03Z
expiresAfter: null
expiresAt: null
errors: []
count: 1
next: null
prev: null
results:
- name: promenade-site-validation
url: https://deckhand/api/v1.0/revisions/4/etc
status: failure
createdAt: 2017-07-16T02:03Z
expiresAfter: null
expiresAt: null
errors: []
...
""")
def test__get_deckhand_validations_empty_errors():
@mock.patch.object(DeckhandClient, 'get_all_revision_validations',
return_value=yaml.safe_load(FK_VAL_RESP_EMPTY.text))
def test__get_deckhand_validations_empty_errors(mock_client):
"""
Tets the functionality of processing a response from deckhand
"""
helper = ConfigdocsHelper(CTX)
helper.deckhand._get_base_validation_resp = (
lambda revision_id: FK_VAL_BASE_RESP)
helper.deckhand._get_subset_validation_response = (
lambda reivsion_id, subset_name: FK_VAL_SUBSET_RESP)
helper.deckhand._get_entry_validation_response = (
lambda reivsion_id, subset_name, entry_id: FK_VAL_ENTRY_RESP_EMPTY)
assert len(helper._get_deckhand_validation_errors(5)) == 0
FK_VAL_BASE_RESP_EMPTY = FakeResponse(
FK_VAL_RESP_EMPTY = FakeResponse(
status_code=200,
text="""
---
@ -841,13 +816,13 @@ results: []
""")
def test__get_deckhand_validation_errors_empty_results():
@mock.patch.object(DeckhandClient, 'get_all_revision_validations',
return_value=yaml.safe_load(FK_VAL_RESP_EMPTY.text))
def test__get_deckhand_validation_errors_empty_results(mock_client):
"""
Tets the functionality of processing a response from deckhand
"""
helper = ConfigdocsHelper(CTX)
helper.deckhand._get_base_validation_resp = (
lambda revision_id: FK_VAL_BASE_RESP_EMPTY)
assert len(helper._get_deckhand_validation_errors(5)) == 0