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:
parent
e11c621a84
commit
6f8b9f858a
@ -41,11 +41,7 @@ class DeckhandPaths(enum.Enum):
|
|||||||
REVISION_LIST = '/revisions'
|
REVISION_LIST = '/revisions'
|
||||||
REVISION_TAG_LIST = '/revisions/{}/tags'
|
REVISION_TAG_LIST = '/revisions/{}/tags'
|
||||||
REVISION_TAG = '/revisions/{}/tags/{}'
|
REVISION_TAG = '/revisions/{}/tags/{}'
|
||||||
REVISION_VALIDATION_LIST = '/revisions/{}/validations'
|
REVISION_VALIDATIONS_LIST = '/revisions/{}/validations/detail'
|
||||||
REVISION_VALIDATION = '/revisions/{}/validations/{}'
|
|
||||||
REVISION_VALIDATION_ENTRY = (
|
|
||||||
'/revisions/{}/validations/{}/entries/{}'
|
|
||||||
)
|
|
||||||
ROLLBACK = '/rollback/{}'
|
ROLLBACK = '/rollback/{}'
|
||||||
|
|
||||||
|
|
||||||
@ -246,98 +242,26 @@ class DeckhandClient(object):
|
|||||||
self._handle_bad_response(response)
|
self._handle_bad_response(response)
|
||||||
return response.text
|
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):
|
def get_all_revision_validations(self, revision_id):
|
||||||
"""
|
"""
|
||||||
Collects and returns the yamls of the validations for a
|
Collects a YAML document containing a list of validation results
|
||||||
revision
|
corresponding to a Deckhand revision ID.
|
||||||
"""
|
|
||||||
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
|
|
||||||
|
|
||||||
def _get_base_validation_resp(self, revision_id):
|
:param revision_id: A Deckhand revision ID corresponding to a set of
|
||||||
# wraps getting the base validation response
|
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(
|
url = DeckhandClient.get_path(
|
||||||
DeckhandPaths.REVISION_VALIDATION_LIST
|
DeckhandPaths.REVISION_VALIDATIONS_LIST
|
||||||
).format(revision_id)
|
).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):
|
return yaml.safe_load(response.text)
|
||||||
# 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)
|
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _handle_bad_response(response, threshold=400):
|
def _handle_bad_response(response, threshold=400):
|
||||||
|
@ -734,24 +734,7 @@ def test_generate_dh_val_message():
|
|||||||
assert generated == expected
|
assert generated == expected
|
||||||
|
|
||||||
|
|
||||||
FK_VAL_BASE_RESP = FakeResponse(
|
FK_VAL_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(
|
|
||||||
status_code=200,
|
status_code=200,
|
||||||
text="""
|
text="""
|
||||||
---
|
---
|
||||||
@ -759,77 +742,69 @@ count: 1
|
|||||||
next: null
|
next: null
|
||||||
prev: null
|
prev: null
|
||||||
results:
|
results:
|
||||||
- id: 0
|
- name: promenade-site-validation
|
||||||
url: https://deckhand/a/url/too/long/for/pep8
|
url: https://deckhand/api/v1.0/revisions/4/etc
|
||||||
status: failure
|
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
|
Tets the functionality of processing a response from deckhand
|
||||||
"""
|
"""
|
||||||
helper = ConfigdocsHelper(CTX)
|
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
|
assert len(helper._get_deckhand_validation_errors(5)) == 2
|
||||||
|
|
||||||
|
|
||||||
FK_VAL_ENTRY_RESP_EMPTY = FakeResponse(
|
FK_VAL_RESP_EMPTY = FakeResponse(
|
||||||
status_code=200,
|
status_code=200,
|
||||||
text="""
|
text="""
|
||||||
---
|
---
|
||||||
name: promenade-site-validation
|
count: 1
|
||||||
url: https://deckhand/a/url/too/long/for/pep8
|
next: null
|
||||||
status: failure
|
prev: null
|
||||||
createdAt: 2017-07-16T02:03Z
|
results:
|
||||||
expiresAfter: null
|
- name: promenade-site-validation
|
||||||
expiresAt: null
|
url: https://deckhand/api/v1.0/revisions/4/etc
|
||||||
errors: []
|
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
|
Tets the functionality of processing a response from deckhand
|
||||||
"""
|
"""
|
||||||
helper = ConfigdocsHelper(CTX)
|
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
|
assert len(helper._get_deckhand_validation_errors(5)) == 0
|
||||||
|
|
||||||
|
|
||||||
FK_VAL_BASE_RESP_EMPTY = FakeResponse(
|
FK_VAL_RESP_EMPTY = FakeResponse(
|
||||||
status_code=200,
|
status_code=200,
|
||||||
text="""
|
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
|
Tets the functionality of processing a response from deckhand
|
||||||
"""
|
"""
|
||||||
helper = ConfigdocsHelper(CTX)
|
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
|
assert len(helper._get_deckhand_validation_errors(5)) == 0
|
||||||
|
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user