From 06de84e0ab4c942dd2fb8cdde29553512a5643e1 Mon Sep 17 00:00:00 2001 From: Bryan Strassner Date: Thu, 4 Oct 2018 16:14:19 -0500 Subject: [PATCH] Add notes processing to the Shipyard API+CLI Enhance the Shipyard API and CLI to retrieve notes that have been specified against actions and steps. Includes a new reusable parameter for verbosity. Change-Id: I1c7f47c0346ce783dacd62b8bbc1fd35a0bf285b --- doc/source/API.rst | 16 ++ doc/source/CLI.rst | 38 ++++- doc/source/_static/shipyard.conf.sample | 9 + .../common/notes/notes_helper.py | 157 +++++++++++++++--- .../common/notes/storage_impl_db.py | 6 +- .../common/notes/storage_impl_mem.py | 7 +- .../control/action/actions_api.py | 20 ++- .../control/action/actions_id_api.py | 29 +++- .../control/action/actions_steps_id_api.py | 30 +++- .../shipyard_airflow/control/api.py | 3 + .../shipyard_airflow/control/base.py | 2 + .../control/helpers/action_helper.py | 35 +++- .../control/middleware/common_params.py | 64 +++++++ .../tests/unit/control/test_actions_api.py | 4 +- .../tests/unit/control/test_actions_id_api.py | 30 +++- .../unit/control/test_actions_steps_id_api.py | 26 ++- .../shipyard_client/api_client/base_client.py | 2 + .../api_client/shipyardclient_context.py | 19 ++- .../shipyard_client/cli/action.py | 3 +- .../shipyard_client/cli/cli_format_common.py | 60 ++++++- .../shipyard_client/cli/commands.py | 14 +- .../shipyard_client/cli/describe/actions.py | 11 +- .../shipyard_client/cli/format_utils.py | 38 +++++ .../cli/describe/test_describe_actions.py | 91 +++++++++- .../tests/unit/cli/get/test_get_actions.py | 99 ++++++++++- .../tests/unit/cli/test_shipyard_commands.py | 3 +- 26 files changed, 716 insertions(+), 100 deletions(-) create mode 100644 src/bin/shipyard_airflow/shipyard_airflow/control/middleware/common_params.py diff --git a/doc/source/API.rst b/doc/source/API.rst index fdc71ab9..6fa3ee55 100644 --- a/doc/source/API.rst +++ b/doc/source/API.rst @@ -31,6 +31,22 @@ Standards used by the API ------------------------- See `API Conventions`_ +Query Parameters +~~~~~~~~~~~~~~~~ + +Query parameters are mostly specific to a Shipyard API resource, but the +following are reused to provide a more consistent interface: + +verbosity + ``?verbosity=1`` + + Provides the user some control over the level of details provided in a + response, with values ranging from 0 (none) to 5 (most). Only some resources + are affected by setting verbosity, but all resources will accept the + parameter. Setting the verbosity parameter to 0 will instruct the resource to + turn off all optional data being returned. The default verbosity level is 1 + (summary). + Notes on examples ----------------- Examples assume the following environment variables are set before diff --git a/doc/source/CLI.rst b/doc/source/CLI.rst index aa9c2b82..853cd17c 100644 --- a/doc/source/CLI.rst +++ b/doc/source/CLI.rst @@ -47,7 +47,7 @@ These OpenStack identity variables are not supported by shipyard. Shipyard command options ------------------------ The base shipyard command supports options that determine cross-CLI behaviors. -These options are positionally immediately following the shipyard command as +These options are positioned immediately following the shipyard command as shown here: :: @@ -59,6 +59,7 @@ shown here: [--debug/--no-debug] [--os-{various}=] [--output-format=[format | raw | cli]] (default = cli) + [--verbosity=[0-5] (default = 1) @@ -95,6 +96,16 @@ shown here: Display results in a plain text interpretation of the response from the invoked Shipyard API. +\--verbosity=<0-5> + Integer value specifying the level of verbosity for the response information + gathered from the API server. Setting a verbosity of ``0`` will remove all + additional information from the response, a verbosity setting of ``1`` will + include summary level notes and information, and ``5`` will include all + available information. This setting does not necessarily effect all of the + CLI commands, but may be set on all invocations. A default value of ``1`` is + used if not specified. + + Commit Commands --------------- @@ -415,20 +426,26 @@ Sample Context Marker: 71d4112e-8b6d-44e8-9617-d9587231ffba User: shipyard - Steps Index State + Steps Index State Notes step/01BZZK07NF04XPC5F4SCTHNPKN/action_xcom 1 success step/01BZZK07NF04XPC5F4SCTHNPKN/dag_concurrency_check 2 success - step/01BZZK07NF04XPC5F4SCTHNPKN/deckhand_get_design_version 3 failed + step/01BZZK07NF04XPC5F4SCTHNPKN/deckhand_get_design_version 3 failed (1) step/01BZZK07NF04XPC5F4SCTHNPKN/validate_site_design 4 None step/01BZZK07NF04XPC5F4SCTHNPKN/deckhand_get_design_version 5 failed step/01BZZK07NF04XPC5F4SCTHNPKN/deckhand_get_design_version 6 failed step/01BZZK07NF04XPC5F4SCTHNPKN/drydock_build 7 None + (1): + + step metadata: deckhand_get_design_version(2017-11-27 20:34:34.443053): Unable to determine version + Commands User Datetime invoke shipyard 2017-11-27 20:34:34.443053+00:00 Validations: None + Notes: + action metadata: 01BZZK07NF04XPC5F4SCTHNPKN(2017-11-27 20:34:24.610604): Invoked using revision 3 describe step ~~~~~~~~~~~~~ @@ -573,9 +590,18 @@ Sample :: $ shipyard get actions - Name Action Lifecycle - deploy_site action/01BZZK07NF04XPC5F4SCTHNPKN Failed - update_site action/01BZZKMW60DV2CJZ858QZ93HRS Processing + Name Action Lifecycle Execution Time Step Succ/Fail/Oth Notes + deploy_site action/01BTP9T2WCE1PAJR2DWYXG805V Failed 2017-09-23T02:42:12 12/1/3 (1) + update_site action/01BZZKMW60DV2CJZ858QZ93HRS Processing 2017-09-23T04:12:21 6/0/10 (2) + + (1): + + action metadata:01BTP9T2WCE1PAJR2DWYXG805V(2017-09-23 02:42:23.346534): Invoked with revision 3 + + (2): + + action metadata:01BZZKMW60DV2CJZ858QZ93HRS(2017-09-23 04:12:31.465342): Invoked with revision 4 + get configdocs ~~~~~~~~~~~~~~ diff --git a/doc/source/_static/shipyard.conf.sample b/doc/source/_static/shipyard.conf.sample index f094fad5..e90ff37e 100644 --- a/doc/source/_static/shipyard.conf.sample +++ b/doc/source/_static/shipyard.conf.sample @@ -68,6 +68,9 @@ # The directory containing the alembic.ini file (string value) #alembic_ini_path = /home/shipyard/shipyard +# Enable profiling of API requests. Do NOT use in production. (boolean value) +#profiler = false + [deckhand] @@ -315,6 +318,12 @@ # Timeout value for http requests (integer value) #timeout = +# Collect per-API call timing information. (boolean value) +#collect_timing = false + +# Log requests to multiple loggers. (boolean value) +#split_loggers = false + [logging] diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py index bbdb7ec0..6361125b 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py @@ -22,13 +22,40 @@ LOG = logging.getLogger(__name__) # Constants and magic numbers for actions: # [7:33] to slice a string like: -# action/12345678901234567890123456 +# +# Text: action/12345678901234567890123456 +# | | +# Position: 0....5.|..1....1....2....2....3..|.3 +# | 0 5 0 5 0 | 5 +# | | +# (7) ACTION_ID_START | +# (33) ACTION_ID_END +# # matching the patterns in this helper. +# ACTION_KEY_PATTERN = "action/{}" ACTION_LOOKUP_PATTERN = "action/" ACTION_ID_START = 7 ACTION_ID_END = 33 +# Constants and magic numbers for steps: +# [32:] to slice a step name pattern +# step/{action_id}/{step_name} +# +# Text: step/12345678901234567890123456/my_step +# Position: 0....5....1....1....2....2....3||..3....4 +# | 0 5 0 5 0|| 5 0 +# | |\ +# (5) STEP_ACTION_ID_START | \ +# | (32) STEP_ID_START +# (31) STEP_ACTION_ID_END +# +STEP_KEY_PATTERN = "step/{}/{}" +STEP_LOOKUP_PATTERN = "step/{}" +STEP_ACTION_ID_START = 5 +STEP_ACTION_ID_END = 31 +STEP_ID_START = 32 + class NotesHelper: """Notes Helper @@ -40,7 +67,8 @@ class NotesHelper: self.nm = notes_manager def _failsafe_make_note(self, assoc_id, subject, sub_type, note_val, - verbosity=None, link_url=None, is_auth_link=None): + verbosity=MIN_VERBOSITY, link_url=None, + is_auth_link=None): """LOG and continue on any note creation failure""" try: self.nm.create( @@ -60,11 +88,13 @@ class NotesHelper: ) LOG.exception(ex) - def _failsafe_get_notes(self, assoc_id_pattern, max_verbosity, + def _failsafe_get_notes(self, assoc_id_pattern, verbosity, exact_match): """LOG and continue on any note retrieval failure""" try: - q = Query(assoc_id_pattern, max_verbosity, exact_match) + if verbosity < MIN_VERBOSITY: + return [] + q = Query(assoc_id_pattern, verbosity, exact_match) return self.nm.retrieve(q) except Exception as ex: LOG.warn( @@ -75,8 +105,12 @@ class NotesHelper: LOG.exception(ex) return [] + # + # Action notes helper methods + # + def make_action_note(self, action_id, note_val, subject=None, - sub_type=None, verbosity=None, link_url=None, + sub_type=None, verbosity=MIN_VERBOSITY, link_url=None, is_auth_link=None): """Creates an action note using a convention for the note's assoc_id @@ -99,8 +133,6 @@ class NotesHelper: subject = action_id if sub_type is None: sub_type = "action metadata" - if verbosity is None: - verbosity = 1 self._failsafe_make_note( assoc_id=assoc_id, @@ -112,48 +144,119 @@ class NotesHelper: is_auth_link=is_auth_link, ) - def get_all_action_notes(self, verbosity=None): + def get_all_action_notes(self, verbosity=MIN_VERBOSITY): """Retrieve notes for all actions, in a dictionary keyed by action id. - :param verbosity: optional, 1-5, the maximum verbosity level to - retrieve, defaults to 1 (most summary level) + :param verbosity: optional integer, 0-5, the maximum verbosity level + to retrieve, defaults to 1 (most summary level) + if set to less than 1, returns {}, skipping any retrieval Warning: if there are a lot of URL links in notes, this could take a long time. The default verbosity of 1 attempts to avoid this as there is less expectation of URL links on summary notes. """ - max_verbosity = verbosity or MIN_VERBOSITY notes = self._failsafe_get_notes( assoc_id_pattern=ACTION_LOOKUP_PATTERN, - max_verbosity=verbosity, + verbosity=verbosity, exact_match=False ) note_dict = {} for n in notes: - # magic numbers [7:33] to slice a string like: - # action/12345678901234567890123456/something - # matching the convention in this helper. - # in the case where there are non-compliant, the slice will make - # the action_id a garbage key and that note will not be easily - # associated. action_id = n.assoc_id[ACTION_ID_START:ACTION_ID_END] if action_id not in note_dict: note_dict[action_id] = [] note_dict[action_id].append(n) return note_dict - def get_action_notes(self, action_id, verbosity=None): + def get_action_notes(self, action_id, verbosity=MAX_VERBOSITY): """Retrive notes related to a particular action :param action_id: the action for which to retrieve notes. - :param verbosity: optional, 1-5, the maximum verbosity level to - retrieve, defaults to 5 (most detailed level) + :param verbosity: optional integer, 0-5, the maximum verbosity level + to retrieve, defaults to 5 (most detailed level) + if set to less than 1, returns [], skipping any retrieval + """ - assoc_id_pattern = ACTION_KEY_PATTERN.format(action_id) - max_verbosity = verbosity or MAX_VERBOSITY - exact_match = True return self._failsafe_get_notes( - assoc_id_pattern=assoc_id_pattern, - max_verbosity=max_verbosity, - exact_match=exact_match + assoc_id_pattern=ACTION_KEY_PATTERN.format(action_id), + verbosity=verbosity, + exact_match=True + ) + + # + # Step notes helper methods + # + + def make_step_note(self, action_id, step_id, note_val, subject=None, + sub_type=None, verbosity=MIN_VERBOSITY, link_url=None, + is_auth_link=None): + """Creates an action note using a convention for the note's assoc_id + + :param action_id: the ULID id of the action containing the note + :param step_id: the step for this note + :param note_val: the text for the note + :param subject: optional subject for the note. Defaults to the + step_id + :param sub_type: optional subject type for the note, defaults to + "step metadata" + :param verbosity: optional verbosity for the note, defaults to 1, + i.e.: summary level + :param link_url: optional link URL if there's additional information + to retreive from another source. + :param is_auth_link: optional, defaults to False, indicating if there + is a need to send a Shipyard service account token with the + request to the optional URL + """ + assoc_id = STEP_KEY_PATTERN.format(action_id, step_id) + if subject is None: + subject = step_id + if sub_type is None: + sub_type = "step metadata" + + self._failsafe_make_note( + assoc_id=assoc_id, + subject=subject, + sub_type=sub_type, + note_val=note_val, + verbosity=verbosity, + link_url=link_url, + is_auth_link=is_auth_link, + ) + + def get_all_step_notes_for_action(self, action_id, + verbosity=MIN_VERBOSITY): + """Retrieve a dict keyed by step names for the action_id + + :param action_id: the action that contains the target steps + :param verbosity: optional integer, 0-5, the maximum verbosity level + to retrieve, defaults to 1 (most summary level) + if set to less than 1, returns {}, skipping any retrieval + """ + notes = self._failsafe_get_notes( + assoc_id_pattern=STEP_LOOKUP_PATTERN.format(action_id), + verbosity=verbosity, + exact_match=False + ) + note_dict = {} + for n in notes: + step_id = n.assoc_id[STEP_ID_START:] + if step_id not in note_dict: + note_dict[step_id] = [] + note_dict[step_id].append(n) + return note_dict + + def get_step_notes(self, action_id, step_id, verbosity=MAX_VERBOSITY): + """Retrive notes related to a particular step + + :param action_id: the action containing the step + :param step_id: the id of the step + :param verbosity: optional integer, 0-5, the maximum verbosity level + to retrieve, defaults to 5 (most detailed level) + if set to less than 1, returns [], skipping any retrieval + + """ + return self._failsafe_get_notes( + assoc_id_pattern=STEP_KEY_PATTERN.format(action_id, step_id), + verbosity=verbosity, + exact_match=True ) diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py index c7577317..ead5a70f 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py @@ -118,7 +118,7 @@ class ShipyardSQLNotesStorage(NotesStorage): def retrieve(self, query): a_id_pat = query.assoc_id_pattern - m_verb = query.max_verbosity + max_verb = query.max_verbosity r_notes = [] with self.session_scope() as session: notes_res = [] @@ -126,14 +126,14 @@ class ShipyardSQLNotesStorage(NotesStorage): n_qry = session.query(TNote).filter( and_( TNote.assoc_id == a_id_pat, - TNote.verbosity <= m_verb + TNote.verbosity <= max_verb ) ) else: n_qry = session.query(TNote).filter( and_( TNote.assoc_id.like(a_id_pat + '%'), - TNote.verbosity <= m_verb + TNote.verbosity <= max_verb ) ) db_notes = n_qry.all() diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py index 947bd0e4..76b3aab6 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py @@ -30,14 +30,15 @@ class MemoryNotesStorage(NotesStorage): def retrieve(self, query): pat = query.assoc_id_pattern - mv = query.max_verbosity + max_verb = query.max_verbosity notes = [] if query.exact_match: for note in self.storage.values(): - if note.assoc_id == pat and note.verbosity <= mv: + if note.assoc_id == pat and note.verbosity <= max_verb: notes.append(note) else: for note in self.storage.values(): - if note.assoc_id.startswith(pat) and note.verbosity <= mv: + if (note.assoc_id.startswith(pat) and + note.verbosity <= max_verb): notes.append(note) return notes diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py index 7aa350d1..e673bdd0 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py @@ -111,7 +111,9 @@ class ActionsResource(BaseResource): Return actions that have been invoked through shipyard. :returns: a json array of action entities """ - resp.body = self.to_json(self.get_all_actions()) + resp.body = self.to_json(self.get_all_actions( + verbosity=req.context.verbosity) + ) resp.status = falcon.HTTP_200 @policy.ApiEnforcer(policy.CREATE_ACTION) @@ -203,8 +205,12 @@ class ActionsResource(BaseResource): return action - def get_all_actions(self): - """ + def get_all_actions(self, verbosity): + """Retrieve all actions known to Shipyard + + :param verbosity: Integer 0-5, the level of verbosity applied to the + response's notes. + Interacts with airflow and the shipyard database to return the list of actions invoked through shipyard. """ @@ -214,7 +220,7 @@ class ActionsResource(BaseResource): all_dag_runs = self.get_dag_run_map() all_tasks = self.get_all_tasks_db() - notes = notes_helper.get_all_action_notes(verbosity=1) + notes = notes_helper.get_all_action_notes(verbosity=verbosity) # correlate the actions and dags into a list of action entites actions = [] @@ -234,7 +240,11 @@ class ActionsResource(BaseResource): step['execution_date'].strftime( '%Y-%m-%dT%H:%M:%S') == dag_key_date ] - action['steps'] = format_action_steps(action_id, action_tasks) + action['steps'] = format_action_steps( + action_id=action_id, + steps=action_tasks, + verbosity=0 + ) action['notes'] = [] for note in notes.get(action_id, []): action['notes'].append(note.view()) diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_id_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_id_api.py index c71737cc..f03d9ca3 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_id_api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_id_api.py @@ -19,6 +19,8 @@ from shipyard_airflow.control.helpers.action_helper import ( determine_lifecycle, format_action_steps ) +from shipyard_airflow.common.notes.notes import MIN_VERBOSITY +from shipyard_airflow.control.helpers.notes import NOTES as notes_helper from shipyard_airflow.db.db import AIRFLOW_DB, SHIPYARD_DB from shipyard_airflow.errors import ApiError @@ -34,13 +36,21 @@ class ActionsIdResource(BaseResource): Return actions that have been invoked through shipyard. :returns: a json array of action entities """ - resp.body = self.to_json(self.get_action(kwargs['action_id'])) + resp.body = self.to_json(self.get_action( + action_id=kwargs['action_id'], + verbosity=req.context.verbosity + )) resp.status = falcon.HTTP_200 - def get_action(self, action_id): + def get_action(self, action_id, verbosity): """ Interacts with airflow and the shipyard database to return the requested action invoked through shipyard. + :param action_id: the action_id to look up + :param verbosity: the maximum verbosity for the associated action. + note that the associated steps will only support a verbosity + of 1 when retrieving an action (but support more verbosity when + retreiving the step itself) """ # get the action from shipyard db action = self.get_action_db(action_id=action_id) @@ -61,9 +71,22 @@ class ActionsIdResource(BaseResource): # put the values together into an "action" object action['dag_status'] = dag['state'] action['action_lifecycle'] = determine_lifecycle(dag['state']) - action['steps'] = format_action_steps(action_id, steps) + step_verbosity = MIN_VERBOSITY if ( + verbosity > MIN_VERBOSITY) else verbosity + action['steps'] = format_action_steps( + action_id=action_id, + steps=steps, + verbosity=step_verbosity + ) action['validations'] = self.get_validations_db(action_id) action['command_audit'] = self.get_action_command_audit_db(action_id) + notes = notes_helper.get_action_notes( + action_id=action_id, + verbosity=verbosity + ) + action['notes'] = [] + for note in notes: + action['notes'].append(note.view()) return action def get_dag_run_by_id(self, dag_id, execution_date): diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_steps_id_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_steps_id_api.py index 94e8a0a1..60535b75 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_steps_id_api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_steps_id_api.py @@ -15,6 +15,8 @@ import falcon from shipyard_airflow import policy from shipyard_airflow.control.base import BaseResource +from shipyard_airflow.common.notes.notes import MAX_VERBOSITY +from shipyard_airflow.control.helpers.notes import NOTES as notes_helper from shipyard_airflow.db.db import AIRFLOW_DB, SHIPYARD_DB from shipyard_airflow.errors import ApiError @@ -31,11 +33,22 @@ class ActionsStepsResource(BaseResource): :returns: a json object describing a step """ resp.body = self.to_json( - self.get_action_step(kwargs['action_id'], kwargs['step_id'])) + self.get_action_step( + action_id=kwargs['action_id'], + step_id=kwargs['step_id'], + verbosity=req.context.verbosity + ) + ) resp.status = falcon.HTTP_200 - def get_action_step(self, action_id, step_id): - """ + def get_action_step(self, action_id, step_id, verbosity=MAX_VERBOSITY): + """Retrieve a single step + + :param action_id: the action_id containing the target step + :param step_id: the step to retrieve + :param verbosity: the level of detail to return for the step. Defaults + to the highest level of detail. + Interacts with airflow and the shipyard database to return the requested step invoked through shipyard. """ @@ -53,12 +66,17 @@ class ActionsStepsResource(BaseResource): # get the action steps from shipyard db steps = self.get_tasks_db(dag_id, dag_execution_date) - + step_notes = notes_helper.get_step_notes( + action_id=action_id, + step_id=step_id, + verbosity=verbosity + ) for idx, step in enumerate(steps): if step_id == step['task_id']: - # TODO (Bryan Strassner) more info about the step? - # like logs? Need requirements defined step['index'] = idx + 1 + step['notes'] = [] + for note in step_notes: + step['notes'].append(note.view()) return step # if we didn't find it, 404 diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/api.py index d7016014..22b0893b 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/api.py @@ -38,6 +38,8 @@ from shipyard_airflow.control.configdocs.rendered_configdocs_api import \ RenderedConfigDocsResource from shipyard_airflow.control.health import HealthResource from shipyard_airflow.control.middleware.auth import AuthMiddleware +from shipyard_airflow.control.middleware.common_params import \ + CommonParametersMiddleware from shipyard_airflow.control.middleware.context import ContextMiddleware from shipyard_airflow.control.middleware.logging_mw import LoggingMiddleware from shipyard_airflow.control.status.status_api import StatusResource @@ -52,6 +54,7 @@ def start_api(): AuthMiddleware(), ContextMiddleware(), LoggingMiddleware(), + CommonParametersMiddleware() ] control_api = falcon.API( request_type=ShipyardRequest, middleware=middlewares) diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/base.py b/src/bin/shipyard_airflow/shipyard_airflow/control/base.py index e23c4b55..21e23865 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/base.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/base.py @@ -19,6 +19,7 @@ import falcon import falcon.request as request import falcon.routing as routing +from shipyard_airflow.common.notes.notes import MIN_VERBOSITY from shipyard_airflow.control.json_schemas import validate_json from shipyard_airflow.errors import InvalidFormatError @@ -104,6 +105,7 @@ class ShipyardRequestContext(object): self.project_domain_id = None # Domain owning project self.is_admin_project = False self.authenticated = False + self.verbosity = MIN_VERBOSITY def set_user(self, user): self.user = user diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/action_helper.py b/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/action_helper.py index 1fc14f5e..e9f0c637 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/action_helper.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/action_helper.py @@ -16,10 +16,11 @@ from datetime import datetime import falcon import logging +from shipyard_airflow.common.notes.notes import MIN_VERBOSITY +from shipyard_airflow.control.helpers.notes import NOTES as notes_helper from shipyard_airflow.db.db import AIRFLOW_DB, SHIPYARD_DB from shipyard_airflow.errors import ApiError - LOG = logging.getLogger(__name__) @@ -49,25 +50,43 @@ def determine_lifecycle(dag_status=None): return lifecycle -def format_action_steps(action_id, steps): - """ Converts a list of action step db records to desired format """ +def format_action_steps(action_id, steps, verbosity=MIN_VERBOSITY): + """ Converts a list of action step db records to desired format + + :param action_id: the action containing steps + :param steps: the list of dictionaries of step info, in database format + :param verbosity: the verbosity level of notes to retrieve, defaults to 1. + if set to a value less than 1, notes will not be retrieved. + """ if not steps: return [] steps_response = [] + step_notes_dict = notes_helper.get_all_step_notes_for_action( + action_id=action_id, + verbosity=verbosity + ) for idx, step in enumerate(steps): - steps_response.append(format_step(action_id=action_id, - step=step, - index=idx + 1)) + step_task_id = step.get('task_id') + steps_response.append( + format_step( + action_id=action_id, + step=step, + index=idx + 1, + notes=[ + note.view() + for note in step_notes_dict.get(step_task_id, []) + ])) return steps_response -def format_step(action_id, step, index): +def format_step(action_id, step, index, notes): """ reformat a step (dictionary) into a common response format """ return { 'url': '/actions/{}/steps/{}'.format(action_id, step.get('task_id')), 'state': step.get('state'), 'id': step.get('task_id'), - 'index': index + 'index': index, + 'notes': notes } diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/middleware/common_params.py b/src/bin/shipyard_airflow/shipyard_airflow/control/middleware/common_params.py new file mode 100644 index 00000000..86d5fce5 --- /dev/null +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/middleware/common_params.py @@ -0,0 +1,64 @@ +# Copyright 2018 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" Common Parameter processing middleware + +Extracts some common parameters from all requests and sets the value (or a +default) on the request context. +The values processed here are those items that have applicability across +multiple endpoints in the API. +This middleware should not be used for endpoint specific values. +""" +import logging + +import falcon + +from shipyard_airflow.common.notes.notes import MAX_VERBOSITY +from shipyard_airflow.errors import ApiError + +LOG = logging.getLogger(__name__) + + +class CommonParametersMiddleware(object): + """Common query parameter processing + + Sets common query parameter values to the request.context in like-named + fields. E.g.: + + ?verbosity=1 results in req.context.verbosity set to the value 1. + """ + def process_request(self, req, resp): + self.verbosity(req) + + def verbosity(self, req): + """Process the verbosity parameter + + :param req: the Falcon request object + Valid values range from 0 (none) to 5 (maximum verbosity) + """ + + try: + verbosity = req.get_param_as_int( + 'verbosity', required=False, min=0, max=MAX_VERBOSITY + ) + if verbosity is not None: + # if not set, retains the context default value. + req.context.verbosity = verbosity + except falcon.HTTPBadRequest as hbr: + LOG.exception(hbr) + raise ApiError( + title="Invalid verbosity parameter", + description=("If specified, verbosity parameter should be a " + "value from 0 to {}".format(MAX_VERBOSITY)), + status=falcon.HTTP_400 + ) diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py b/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py index 04570ec0..0ccfc0df 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py @@ -301,7 +301,7 @@ def test_get_all_actions(*args): action_resource.get_all_actions_db = actions_db action_resource.get_all_dag_runs_db = dag_runs_db action_resource.get_all_tasks_db = tasks_db - result = action_resource.get_all_actions() + result = action_resource.get_all_actions(verbosity=1) assert len(result) == len(actions_db()) for action in result: if action['name'] == 'dag_it': @@ -327,7 +327,7 @@ def test_get_all_actions_notes(*args): nh.make_action_note('aaaaaa', "hello from aaaaaa2") nh.make_action_note('bbbbbb', "hello from bbbbbb") - result = action_resource.get_all_actions() + result = action_resource.get_all_actions(verbosity=1) assert len(result) == len(actions_db()) for action in result: if action['id'] == 'aaaaaa': diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_actions_id_api.py b/src/bin/shipyard_airflow/tests/unit/control/test_actions_id_api.py index b86ff8ca..af7a0a1d 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_actions_id_api.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_actions_id_api.py @@ -16,6 +16,11 @@ from unittest import mock import pytest +from shipyard_airflow.common.notes.notes import NotesManager +from shipyard_airflow.common.notes.notes_helper import NotesHelper +from shipyard_airflow.common.notes.storage_impl_mem import ( + MemoryNotesStorage +) from shipyard_airflow.control.action.actions_id_api import (ActionsIdResource) from shipyard_airflow.control.base import ShipyardRequestContext from shipyard_airflow.policy import ShipyardPolicy @@ -31,6 +36,15 @@ DATE_TWO_STR = DATE_TWO.strftime('%Y-%m-%dT%H:%M:%S') context = ShipyardRequestContext() +def get_token(): + """Stub method to use for NotesHelper/NotesManager""" + return "token" + +# Notes helper that can be mocked into various objects to prevent database +# dependencies +nh = NotesHelper(NotesManager(MemoryNotesStorage(), get_token)) + + def actions_db(action_id): """ replaces the actual db call @@ -151,12 +165,15 @@ def test_on_get(mock_authorize, mock_get_action): action_resource.on_get(req, resp, **kwargs) mock_authorize.assert_called_once_with('workflow_orchestrator:get_action', context) - mock_get_action.assert_called_once_with(kwargs['action_id']) + mock_get_action.assert_called_once_with(action_id=None, verbosity=1) assert resp.body == '"action_returned"' assert resp.status == '200 OK' - -def test_get_action_success(): +@mock.patch('shipyard_airflow.control.helpers.action_helper.notes_helper', + new=nh) +@mock.patch('shipyard_airflow.control.action.actions_id_api.notes_helper', + new=nh) +def test_get_action_success(*args): """ Tests the main response from get all actions """ @@ -168,7 +185,10 @@ def test_get_action_success(): action_resource.get_validations_db = get_validations action_resource.get_action_command_audit_db = get_ac_audit - action = action_resource.get_action('12345678901234567890123456') + action = action_resource.get_action( + action_id='12345678901234567890123456', + verbosity=1 + ) if action['name'] == 'dag_it': assert len(action['steps']) == 3 assert action['dag_status'] == 'FAILED' @@ -182,7 +202,7 @@ def test_get_action_errors(mock_get_action): action_id = '12345678901234567890123456' with pytest.raises(ApiError) as expected_exc: - action_resource.get_action(action_id) + action_resource.get_action(action_id=action_id, verbosity=1) assert action_id in str(expected_exc) assert 'Action not found' in str(expected_exc) diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_actions_steps_id_api.py b/src/bin/shipyard_airflow/tests/unit/control/test_actions_steps_id_api.py index 23fd0c15..1c73918b 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_actions_steps_id_api.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_actions_steps_id_api.py @@ -16,9 +16,14 @@ from unittest.mock import patch import pytest -from shipyard_airflow.errors import ApiError +from shipyard_airflow.common.notes.notes import NotesManager +from shipyard_airflow.common.notes.notes_helper import NotesHelper +from shipyard_airflow.common.notes.storage_impl_mem import ( + MemoryNotesStorage +) from shipyard_airflow.control.action.actions_steps_id_api import \ ActionsStepsResource +from shipyard_airflow.errors import ApiError from tests.unit.control import common DATE_ONE = datetime(2017, 9, 13, 11, 13, 3, 57000) @@ -27,6 +32,15 @@ DATE_ONE_STR = DATE_ONE.strftime('%Y-%m-%dT%H:%M:%S') DATE_TWO_STR = DATE_TWO.strftime('%Y-%m-%dT%H:%M:%S') +def get_token(): + """Stub method to use for NotesHelper/NotesManager""" + return "token" + +# Notes helper that can be mocked into various objects to prevent database +# dependencies +nh = NotesHelper(NotesManager(MemoryNotesStorage(), get_token)) + + def actions_db(action_id): """ replaces the actual db call @@ -99,7 +113,11 @@ class TestActionsStepsResource(): headers=common.AUTH_HEADERS) assert result.status_code == 200 - def test_get_action_step_success(self): + @patch('shipyard_airflow.control.helpers.action_helper.notes_helper', + new=nh) + @patch('shipyard_airflow.control.action.actions_steps_id_api.notes_helper', + new=nh) + def test_get_action_step_success(self, *args): """Tests the main response from get all actions""" action_resource = ActionsStepsResource() # stubs for db @@ -123,6 +141,10 @@ class TestActionsStepsResource(): '59bb330a-9e64-49be-a586-d253bb67d443', 'cheese') assert 'Action not found' in str(api_error) + @patch('shipyard_airflow.control.helpers.action_helper.notes_helper', + new=nh) + @patch('shipyard_airflow.control.action.actions_steps_id_api.notes_helper', + new=nh) def test_get_action_step_error_step(self): """Validate ApiError, 'Step not found' is raised""" action_resource = ActionsStepsResource() diff --git a/src/bin/shipyard_client/shipyard_client/api_client/base_client.py b/src/bin/shipyard_client/shipyard_client/api_client/base_client.py index 43538d20..68570959 100644 --- a/src/bin/shipyard_client/shipyard_client/api_client/base_client.py +++ b/src/bin/shipyard_client/shipyard_client/api_client/base_client.py @@ -86,6 +86,7 @@ class BaseClient(metaclass=abc.ABCMeta): 'content-type': content_type, 'X-Auth-Token': self.get_token() } + query_params['verbosity'] = self.context.verbosity self.debug('Post request url: ' + url) self.debug('Query Params: ' + str(query_params)) # This could use keystoneauth1 session, but that library handles @@ -112,6 +113,7 @@ class BaseClient(metaclass=abc.ABCMeta): 'X-Context-Marker': self.context.context_marker, 'X-Auth-Token': self.get_token() } + query_params['verbosity'] = self.context.verbosity self.debug('url: ' + url) self.debug('Query Params: ' + str(query_params)) response = requests.get(url, params=query_params, headers=headers) diff --git a/src/bin/shipyard_client/shipyard_client/api_client/shipyardclient_context.py b/src/bin/shipyard_client/shipyard_client/api_client/shipyardclient_context.py index d80a977a..eb3520fa 100644 --- a/src/bin/shipyard_client/shipyard_client/api_client/shipyardclient_context.py +++ b/src/bin/shipyard_client/shipyard_client/api_client/shipyardclient_context.py @@ -17,19 +17,22 @@ LOG = logging.getLogger(__name__) class ShipyardClientContext: - """A context object for ShipyardClient instances.""" + """A context object for ShipyardClient instances. - def __init__(self, keystone_auth, context_marker, debug=False): - """Shipyard context object + :param dict keystone_auth: auth_url, password, project_domain_name, + project_name, username, user_domain_name + :param str context_marker: a UUID value used to track a request + :param bool debug: defaults False, enable debugging + :param int verbosity: 0-5, default=1, the level of verbosity to set + for the API + """ - :param bool debug: true, or false - :param str context_marker: - :param dict keystone_auth: auth_url, password, project_domain_name, - project_name, username, user_domain_name - """ + def __init__(self, keystone_auth, context_marker, + debug=False, verbosity=1): self.debug = debug if self.debug: LOG.setLevel(logging.DEBUG) self.keystone_auth = keystone_auth self.context_marker = context_marker + self.verbosity = verbosity diff --git a/src/bin/shipyard_client/shipyard_client/cli/action.py b/src/bin/shipyard_client/shipyard_client/cli/action.py index 6dde3865..b5884bde 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/action.py +++ b/src/bin/shipyard_client/shipyard_client/cli/action.py @@ -109,7 +109,8 @@ class CliAction(AbstractCliAction): self.debug = self.api_parameters.get('debug') self.client_context = ShipyardClientContext( - self.auth_vars, self.context_marker, self.debug) + self.auth_vars, self.context_marker, self.debug, + self.api_parameters.get('verbosity')) def get_api_client(self): """Returns the api client for this action""" diff --git a/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py b/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py index 11500e59..983edbf5 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py +++ b/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py @@ -24,18 +24,36 @@ def gen_action_steps(step_list, action_id): Returns a string representation of the table. """ # Generate the steps table. - steps = format_utils.table_factory(field_names=['Steps', 'Index', 'State']) + steps = format_utils.table_factory( + field_names=['Steps', 'Index', 'State', 'Notes'] + ) + # rendered notes , a list of lists of notes + r_notes = [] + if step_list: for step in step_list: + notes = step.get('notes') + if notes: + r_notes.append(format_utils.format_notes(notes)) steps.add_row([ 'step/{}/{}'.format(action_id, step.get('id')), step.get('index'), - step.get('state') + step.get('state'), + "({})".format(len(r_notes)) if notes else "" ]) else: - steps.add_row(['None', '', '']) + steps.add_row(['None', '', '', '']) - return format_utils.table_get_string(steps) + table_string = format_utils.table_get_string(steps) + + if r_notes: + note_index = 1 + for note_list in r_notes: + table_string += "\n\n({}):\n\n{}".format( + note_index, "\n".join(note_list) + ) + note_index += 1 + return table_string def gen_action_commands(command_list): @@ -123,21 +141,36 @@ def gen_action_table(action_list): """ actions = format_utils.table_factory( field_names=['Name', 'Action', 'Lifecycle', 'Execution Time', - 'Step Succ/Fail/Oth']) + 'Step Succ/Fail/Oth', 'Notes']) + # list of lists of rendered notes + r_notes = [] if action_list: # sort by id, which is ULID - chronological. for action in sorted(action_list, key=lambda k: k['id']): + notes = action.get('notes') + if notes: + r_notes.append(format_utils.format_notes(notes)) actions.add_row([ action.get('name'), 'action/{}'.format(action.get('id')), action.get('action_lifecycle'), action.get('dag_execution_date'), - _step_summary(action.get('steps', [])) + _step_summary(action.get('steps', [])), + "({})".format(len(r_notes)) if notes else "" ]) else: - actions.add_row(['None', '', '', '', '']) + actions.add_row(['None', '', '', '', '', '']) - return format_utils.table_get_string(actions) + table_string = format_utils.table_get_string(actions) + + if r_notes: + note_index = 1 + for note_list in r_notes: + table_string += "\n\n({}):\n\n{}".format( + note_index, "\n".join(note_list) + ) + note_index += 1 + return table_string def _step_summary(step_list): @@ -336,3 +369,14 @@ def _site_statuses_switcher(status_type): call_func = status_func_switcher.get(status_type, lambda: None) return call_func + +def gen_detail_notes(dict_with_notes): + """Generates a standard formatted section of notes + + :param dict_with_notes: a dictionary with a possible notes field. + :returns: string of notes or empty string if there were no notes + """ + n_strings = format_utils.format_notes(dict_with_notes.get('notes', [])) + if n_strings: + return "Notes:\n{}".format("\n".join(n_strings)) + return "" diff --git a/src/bin/shipyard_client/shipyard_client/cli/commands.py b/src/bin/shipyard_client/shipyard_client/cli/commands.py index 472aa2d7..fec0e2ad 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/commands.py @@ -61,17 +61,24 @@ from shipyard_client.cli.input_checks import check_control_action, check_id @click.option( '--os-auth-url', envvar='OS_AUTH_URL', required=False) # Allows context (ctx) to be passed +@click.option( + '--verbosity', + '-v', + required=False, + type=click.IntRange(0, 5), + default=1) @click.pass_context def shipyard(ctx, context_marker, debug, os_project_domain_name, os_user_domain_name, os_project_name, os_username, os_password, - os_auth_url, output_format): + os_auth_url, output_format, verbosity): """ COMMAND: shipyard \n DESCRIPTION: The base shipyard command supports options that determine cross-CLI behaviors. These options are positioned immediately following the shipyard command. \n FORMAT: shipyard [--context-marker=] [--os_{various}=] - [--debug/--no-debug] [--output-format= \n + [--debug/--no-debug] [--output-format=] + \n """ if not ctx.obj: ctx.obj = {} @@ -99,7 +106,8 @@ def shipyard(ctx, context_marker, debug, os_project_domain_name, ctx.obj['API_PARAMETERS'] = { 'auth_vars': auth_vars, 'context_marker': str(context_marker) if context_marker else None, - 'debug': debug + 'debug': debug, + 'verbosity': verbosity, } ctx.obj['FORMAT'] = output_format diff --git a/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py b/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py index d29b8794..44bc4eae 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py +++ b/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py @@ -45,14 +45,15 @@ class DescribeAction(CliAction): """ resp_j = response.json() # Assemble the sections of the action details - return '{}\n\n{}\n\n{}\n\n{}\n'.format( + return '{}\n\n{}\n\n{}\n\n{}\n\n{}\n'.format( cli_format_common.gen_action_details(resp_j), cli_format_common.gen_action_steps(resp_j.get('steps'), resp_j.get('id')), cli_format_common.gen_action_commands(resp_j.get('command_audit')), cli_format_common.gen_action_validations( resp_j.get('validations') - ) + ), + cli_format_common.gen_detail_notes(resp_j) ) @@ -88,8 +89,10 @@ class DescribeStep(CliAction): Handles 200 responses """ resp_j = response.json() - return cli_format_common.gen_action_step_details(resp_j, - self.action_id) + return "{}\n\n{}\n".format( + cli_format_common.gen_action_step_details(resp_j, self.action_id), + cli_format_common.gen_detail_notes(resp_j) + ) class DescribeValidation(CliAction): diff --git a/src/bin/shipyard_client/shipyard_client/cli/format_utils.py b/src/bin/shipyard_client/shipyard_client/cli/format_utils.py index 5ab1c9c6..2f6cc6e8 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/format_utils.py +++ b/src/bin/shipyard_client/shipyard_client/cli/format_utils.py @@ -234,3 +234,41 @@ def table_get_string(table, title='', vertical_char='|', align='l'): # vertical_char - Single character string used to draw vertical # lines. Default is '|'. return table.get_string(title=title, vertical_char=vertical_char) + + +def format_notes(notes): + """Formats a list of notes. + + :param list notes: The list of note dictionaries to display + :returns: a list of note strings + + Assumed note dictionary example: + { + 'assoc_id': "action/12345678901234567890123456, + 'subject': "12345678901234567890123456", + 'sub_type': "action", + 'note_val': "This is the message", + 'verbosity': 1, + 'note_id': "09876543210987654321098765", + 'note_timestamp': "2018-10-08 14:23:53.346534", + 'resolved_url_value': "
some info
+ } + """ + nl = [] + for n in notes: + try: + s = "{}:{}({}): {}".format( + n['sub_type'], + n['subject'], + n['note_timestamp'], + n['note_val'] + ) + if n['resolved_url_value']: + s += "\n >>> {}".format( + n['resolved_url_value'] + ) + except KeyError: + s = "!!! Unparseable Note: {}".format(n) + + nl.append(s) + return nl diff --git a/src/bin/shipyard_client/tests/unit/cli/describe/test_describe_actions.py b/src/bin/shipyard_client/tests/unit/cli/describe/test_describe_actions.py index 913188e5..99c5bf50 100644 --- a/src/bin/shipyard_client/tests/unit/cli/describe/test_describe_actions.py +++ b/src/bin/shipyard_client/tests/unit/cli/describe/test_describe_actions.py @@ -49,10 +49,69 @@ GET_ACTION_API_RESP = """ "id": "action_xcom", "url": "/actions/01BTTMFVDKZFRJM80FGD7J1AKN/steps/action_xcom", "index": 1, - "state": "success" + "state": "success", + "notes": [ + { + "assoc_id": "step/01BTTMFVDKZFRJM80FGD7J1AKN/action_xcom", + "subject": "action_xcom", + "sub_type": "step metadata", + "note_val": "This is a note for the action_xcom", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXY0", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": null + }, + { + "assoc_id": "step/01BTTMFVDKZFRJM80FGD7J1AKN/action_xcom", + "subject": "action_xcom", + "sub_type": "step metadata", + "note_val": "action_xcom really worked", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXY1", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": null + } + ] + }, + { + "id": "part2", + "url": "/actions/01BTTMFVDKZFRJM80FGD7J1AKN/steps/part2", + "index": 2, + "state": "success", + "notes": [] + }, + { + "id": "part3", + "url": "/actions/01BTTMFVDKZFRJM80FGD7J1AKN/steps/part3", + "index": 3, + "state": "success", + "notes": [ + { + "assoc_id": "step/01BTTMFVDKZFRJM80FGD7J1AKN/part3", + "subject": "part3", + "sub_type": "step metadata", + "note_val": "This is a note for the part3", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXY2", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": null + } + ] } ], - "action_lifecycle": "Failed" + "action_lifecycle": "Failed", + "notes": [ + { + "assoc_id": "action/01BTTMFVDKZFRJM80FGD7J1AKN", + "subject": "01BTTMFVDKZFRJM80FGD7J1AKN", + "sub_type": "action metadata", + "note_val": "This is a note for some action", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXYA", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": "Your lucky numbers are 1, 3, 5, and Q" + } + ] } """ @@ -75,6 +134,8 @@ def test_describe_action(*args): assert 'Steps' in response assert 'Commands' in response assert 'Validations:' in response + assert 'This is a note for the part3' in response + assert '>>> Your lucky numbers are 1, 3, 5, and Q' @responses.activate @@ -111,7 +172,29 @@ GET_STEP_API_RESP = """ "execution_date": "2017-09-24 19:05:49", "dag_id": "deploy_site", "index": 1, - "start_date": "2017-09-24 19:05:59.281032" + "start_date": "2017-09-24 19:05:59.281032", + "notes": [ + { + "assoc_id": "step/01BTTMFVDKZFRJM80FGD7J1AKN/preflight", + "subject": "preflight", + "sub_type": "step metadata", + "note_val": "This is a note for the preflight", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXY3", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": null + }, + { + "assoc_id": "step/01BTTMFVDKZFRJM80FGD7J1AKN/preflight", + "subject": "preflight", + "sub_type": "step metadata", + "note_val": "preflight really worked", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXY4", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": null + } + ] } """ @@ -130,6 +213,8 @@ def test_describe_step(*args): '01BTTMFVDKZFRJM80FGD7J1AKN', 'preflight').invoke_and_return_resp() assert 'step/01BTTMFVDKZFRJM80FGD7J1AKN/preflight' in response + assert 'preflight really worked' in response + assert 'This is a note for the preflight' in response @responses.activate diff --git a/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py b/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py index c52c517f..105c35a3 100644 --- a/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py +++ b/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py @@ -43,7 +43,19 @@ GET_ACTIONS_API_RESP = """ "id": "concurrency_check", "url": "/actions/01BTP9T2WCE1PAJR2DWYXG805V/steps/concurrency_check", "index": 2, - "state": "success" + "state": "success", + "notes": [ + { + "assoc_id": "step/01BTP9T2WCE1PAJR2DWYXG805V/concurrency_check", + "subject": "concurrency_check", + "sub_type": "step metadata", + "note_val": "This is a note for the concurrency check", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXYZ", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": null + } + ] }, { "id": "preflight", @@ -59,7 +71,19 @@ GET_ACTIONS_API_RESP = """ "datetime": "2017-09-23 02:42:06.860597+00:00", "user": "shipyard", "context_marker": "416dec4b-82f9-4339-8886-3a0c4982aec3", - "name": "deploy_site" + "name": "deploy_site", + "notes": [ + { + "assoc_id": "action/01BTP9T2WCE1PAJR2DWYXG805V", + "subject": "01BTP9T2WCE1PAJR2DWYXG805V", + "sub_type": "action metadata", + "note_val": "This is a note for some action", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXYA", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": "Your lucky numbers are 1, 3, 5, and Q" + } + ] } ] """ @@ -79,6 +103,77 @@ def test_get_actions(*args): assert 'action/01BTP9T2WCE1PAJR2DWYXG805V' in response assert 'Lifecycle' in response assert '2/1/0' in response + assert 'This is a note for the concurrency check' not in response + assert '>>> Your lucky numbers are 1, 3, 5, and Q' in response + + +GET_ACTIONS_API_RESP_UNPARSEABLE_NOTE = """ +[ + { + "dag_status": "failed", + "parameters": {}, + "steps": [ + { + "id": "action_xcom", + "url": "/actions/01BTP9T2WCE1PAJR2DWYXG805V/steps/action_xcom", + "index": 1, + "state": "success" + } + ], + "action_lifecycle": "Failed", + "dag_execution_date": "2017-09-23T02:42:12", + "id": "01BTP9T2WCE1PAJR2DWYXG805V", + "dag_id": "deploy_site", + "datetime": "2017-09-23 02:42:06.860597+00:00", + "user": "shipyard", + "context_marker": "416dec4b-82f9-4339-8886-3a0c4982aec3", + "name": "deploy_site", + "notes": [ + { + "assoc_id": "action/01BTP9T2WCE1PAJR2DWYXG805V", + "subject": "01BTP9T2WCE1PAJR2DWYXG805V", + "sub_type": "action metadata", + "note_val": "This is the first note for some action", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXA1", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": "Your lucky numbers are 1, 3, 5, and Q" + }, + { + "note_val": "This note is broken" + }, + { + "assoc_id": "action/01BTP9T2WCE1PAJR2DWYXG805V", + "subject": "01BTP9T2WCE1PAJR2DWYXG805V", + "sub_type": "action metadata", + "note_val": "The previous note is bad. It is missing fields", + "verbosity": 1, + "note_id": "ABCDEFGHIJKLMNOPQRSTUVWXA2", + "note_timestamp": "2018-10-08 14:23:53.346534", + "resolved_url_value": null + } + ] + } +] +""" + + +@responses.activate +@mock.patch.object(BaseClient, 'get_endpoint', lambda x: 'http://shiptest') +@mock.patch.object(BaseClient, 'get_token', lambda x: 'abc') +def test_get_actions_unparseable_note(*args): + responses.add( + responses.GET, + 'http://shiptest/actions', + body=GET_ACTIONS_API_RESP_UNPARSEABLE_NOTE, + status=200) + response = GetActions(stubs.StubCliContext()).invoke_and_return_resp() + assert 'deploy_site' in response + assert 'action/01BTP9T2WCE1PAJR2DWYXG805V' in response + assert 'Lifecycle' in response + assert 'This is the first note for some action' in response + assert "{'note_val': 'This note is broken'}" in response + assert 'The previous note is bad' in response @responses.activate diff --git a/src/bin/shipyard_client/tests/unit/cli/test_shipyard_commands.py b/src/bin/shipyard_client/tests/unit/cli/test_shipyard_commands.py index 8982893c..89ff5ee3 100644 --- a/src/bin/shipyard_client/tests/unit/cli/test_shipyard_commands.py +++ b/src/bin/shipyard_client/tests/unit/cli/test_shipyard_commands.py @@ -50,5 +50,6 @@ def test_shipyard(): mock_method.assert_called_once_with( auth_vars, '88888888-4444-4444-4444-121212121212', - True + True, + 1 )