From 5a9abc73dd5b5cbe320499429e1dc56bbdb2927f Mon Sep 17 00:00:00 2001 From: Bryan Strassner Date: Sat, 13 Oct 2018 10:15:16 -0500 Subject: [PATCH] Modify note access methods While iterating on the next steps of using notes, it became clear that several changes to the output and access methods for notes needed enhancements. This change introduces a new way to access a note's URL information via a new API/CLI, while removing the resolution of URLs from the existing note output. This supports the concept of "builddata" coming back with sizes of 800kb or more - which really can never work out inline in other data, especially in cases where there is multiplicity of the information across many items. New API: GET /notedetails/{id} CLI: shipyard get notedetails/{id} and/or shipyard get notedetails {id} Returns the resolution of the URL for a note, outputting the raw info as the response (not structured in a JSON response). The CLI will attempt to minimally format the response if it has inline \n characters by replacing them will real newlines in the output (if the output-format is set to either cli or format. Raw format will be returned as-is. The existing notes responses are changed to not include the resolution of the URL information inline, but rather provide the text: Details at notedetails/{id} The CLI will interpret this and present: - Info available with 'describe notedetails/09876543210987654321098765' This is an attempt to inform the user to access the note details that way - luckily the API and CLI align on the term notedetails, as the word details works well enough in the singular form presented by the CLI and the plural form used by the API. The ID returned is the unique id of the note (ULID format). Notes that have no URL will return a 404 response from the API (and an appropriately formatted value from the CLI). This approach solves an issue beyond the large inline values from URLs; providing a means to NOT resolve the URLs except in a one-at-a-time way. Long lists of notes will no longer have the risk of long waits nor needing of parallelization of retrieval of URLs for notes. This change introduces an API-side sorting of notes by timestamp, providing a chronological presentation of the information that may or may not match the ULID or insertion ordering of the notes. Additional feedback from peers about the output of noted indicated that the CLI formatting of notes in general was in need of visual tuning. As such, this change introduces changes to the formatting of the output of notes from the CLI: - Notes for describing an item will be presented with a more specific header, e.g.: Action Notes: or Step Notes: instead of simply Notes. - Tables with notes will change the header from "Notes" to "Footnotes" give the user a better marker that the notes follow the current table. - Table footnotes will be presented in a table format similar to the following, with headings matching the kind of note being produced. Step Footnotes Note (1) > blah blah blah > yakkity yakkity (2) > stuff stuff stuff stuff stuff stuff stuff stuff stuff stuff - Info available with 'describe notedetails/... > things things things Change-Id: I1680505d5c555b2293419179ade995b0e8484e6d --- charts/shipyard/values.yaml | 1 + doc/source/API.rst | 64 ++++++ doc/source/CLI.rst | 68 ++++-- .../_static/shipyard.policy.yaml.sample | 5 + .../etc/shipyard/policy.yaml.sample | 5 + .../shipyard_airflow/common/notes/errors.py | 25 +++ .../shipyard_airflow/common/notes/notes.py | 110 +++++----- .../common/notes/notes_helper.py | 206 +++++++++++++----- .../common/notes/storage_impl_db.py | 13 +- .../common/notes/storage_impl_mem.py | 8 + .../shipyard_airflow/control/api.py | 2 + .../control/notes/__init__.py | 0 .../control/notes/notedetails_api.py | 105 +++++++++ .../shipyard_airflow/policy.py | 11 + .../tests/unit/common/notes/test_notes.py | 110 ++++++++-- .../api_client/shipyard_api_client.py | 10 + .../shipyard_client/cli/cli_format_common.py | 115 +++++++--- .../shipyard_client/cli/describe/actions.py | 42 +++- .../shipyard_client/cli/describe/commands.py | 42 +++- .../shipyard_client/cli/format_utils.py | 38 ---- .../shipyard_client/cli/input_checks.py | 8 +- .../test_shipyard_api_client.py | 11 + .../tests/unit/cli/get/test_get_actions.py | 4 +- 23 files changed, 766 insertions(+), 237 deletions(-) create mode 100644 src/bin/shipyard_airflow/shipyard_airflow/control/notes/__init__.py create mode 100644 src/bin/shipyard_airflow/shipyard_airflow/control/notes/notedetails_api.py diff --git a/charts/shipyard/values.yaml b/charts/shipyard/values.yaml index 1ea5af98..5ff9477d 100644 --- a/charts/shipyard/values.yaml +++ b/charts/shipyard/values.yaml @@ -372,6 +372,7 @@ conf: workflow_orchestrator:get_renderedconfigdocs: rule:admin_read_access workflow_orchestrator:list_workflows: rule:admin_read_access workflow_orchestrator:get_workflow: rule:admin_read_access + workflow_orchestrator:get_notedetails: rule: admin_read_access workflow_orchestrator:get_site_statuses: rule:admin_read_access workflow_orchestrator:action_deploy_site: rule:admin_create workflow_orchestrator:action_update_site: rule:admin_create diff --git a/doc/source/API.rst b/doc/source/API.rst index 6fa3ee55..130f15b3 100644 --- a/doc/source/API.rst +++ b/doc/source/API.rst @@ -26,6 +26,7 @@ Shipyard functionality: 3. Airflow Monitoring 4. Site Statuses 5. Logs Retrieval +6. Notes Handling Standards used by the API ------------------------- @@ -1205,5 +1206,68 @@ Example [2018-04-11 07:30:43,945] {{base_task_runner.py:98}} INFO - Subtask: [2018-04-11 07:30:43,944] {{python_operator.py:90}} INFO - Done. Returned value was: None [2018-04-11 07:30:43,992] {{base_task_runner.py:98}} INFO - Subtask: """) +Notes Handling API +------------------ +The notes facilities of Shipyard are primarily interwoven in other APIs. This +endpoint adds the ability to retrieve additional information associated with a +note. The first use case for this API is the retrieval of builddata from +Drydock, which can be many hundreds of kilobytes of text. + +/v1.0/notedetails/{note_id} +~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Retrieves the note details that are associated via URL with a note at the time +of note creation. Unlike some responses from Shipyard, this API returns the +remote information as-is, as the response body, without any further wrapping in +a JSON structure. + +Entity Structure +^^^^^^^^^^^^^^^^ +Raw text of the note's associated information. + +GET /v1.0/notedetails/{node_id} +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Looks up the specified note and follows the associated URL to retrieve +information related to the note. + +Query parameters +'''''''''''''''' + +N/A + +Responses +''''''''' + +200 OK + + Accompanied by the text looked up from the note's associated URL + +400 Bad Request + + When the note_id is not a valid ULID value. + +404 Not Found + + When the note does not exist, or the note does not have a URL associated. + +500 Internal Server Error + + When the remote source of the information cannot be accessed, or if there is + a misconfiguration of the type of note preventing appropriate authorization + checking. + +Example +''''''' + +:: + + curl -D - \ + -X GET $URL/api/v1.0/notedetails/01CASSSZT7CP1F0NKHCAJBCJGR \ + -H "X-Auth-Token:$TOKEN" + + HTTP/1.1 200 OK + x-shipyard-req: 49f74418-22b3-4629-8ddb-259bdfccf2fd + + Potentially a lot of information here + .. _API Conventions: https://airshipit.readthedocs.io/en/latest/api-conventions.html diff --git a/doc/source/CLI.rst b/doc/source/CLI.rst index 853cd17c..b166503b 100644 --- a/doc/source/CLI.rst +++ b/doc/source/CLI.rst @@ -383,6 +383,10 @@ Retrieves the detailed information about the supplied namespaced item Equivalent to: shipyard describe action 01BTG32JW87G0YKA1K29TKNAFX + shipyard describe notedetails/01BTG32JW87G0YKA1KA9TBNA32 + Equivalent to: + shipyard describe notedetails 01BTG32JW87G0YKA1KA9TBNA32 + shipyard describe step/01BTG32JW87G0YKA1K29TKNAFX/preflight Equivalent to: shipyard describe step preflight --action=01BTG32JW87G0YKA1K29TKNAFX @@ -413,7 +417,6 @@ Retrieves the detailed information about the supplied action id. Sample ^^^^^^ - :: $ shipyard describe action/01BZZK07NF04XPC5F4SCTHNPKN @@ -426,7 +429,7 @@ Sample Context Marker: 71d4112e-8b6d-44e8-9617-d9587231ffba User: shipyard - Steps Index State Notes + Steps Index State Footnotes step/01BZZK07NF04XPC5F4SCTHNPKN/action_xcom 1 success step/01BZZK07NF04XPC5F4SCTHNPKN/dag_concurrency_check 2 success step/01BZZK07NF04XPC5F4SCTHNPKN/deckhand_get_design_version 3 failed (1) @@ -435,17 +438,39 @@ Sample 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 + Step Footnotes Note + (1) > step metadata: deckhand_get_design_version(2017-11-27 20:34:34.443053): Unable to determine version + - Info available with 'describe notedetails/09876543210987654321098765' 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 + Action Notes: + > action metadata: 01BZZK07NF04XPC5F4SCTHNPKN(2017-11-27 20:34:24.610604): Invoked using revision 3 + +describe notedetails +~~~~~~~~~~~~~~~~~~~~ +Retrieves extended information related to a note. + +:: + + shipyard describe notedetails + + Example: + shipyard describe notedetails/01BTG32JW87G0YKA1KA9TBNA32 + + + The id of the note referenced as having more details in a separate response + +Sample +^^^^^^ + +:: + + $ shipyard describe notedetails/01BTG32JW87G0YKA1KA9TBNA32 + describe step ~~~~~~~~~~~~~ @@ -469,7 +494,6 @@ Retrieves the step details associated with an action and step. Sample ^^^^^^ - :: $ shipyard describe step/01BZZK07NF04XPC5F4SCTHNPKN/action_xcom @@ -483,6 +507,10 @@ Sample Try Number: 1 Operator: PythonOperator + Step Notes: + > step metadata: deckhand_get_design_version(2017-11-27 20:34:34.443053): Unable to determine version + - Info available with 'describe notedetails/09876543210987654321098765' + describe validation ~~~~~~~~~~~~~~~~~~~ @@ -508,7 +536,6 @@ validation id Sample ^^^^^^ - :: TBD @@ -533,7 +560,6 @@ workflow engine. Sample ^^^^^^ - :: $ shipyard describe workflow deploy_site__2017-11-27T20:34:33.000000 @@ -590,17 +616,13 @@ Sample :: $ shipyard get actions - 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) + Name Action Lifecycle Execution Time Step Succ/Fail/Oth Footnotes + 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 + Action Footnotes Note + (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 @@ -653,8 +675,8 @@ differences between the 'committed' and 'buffer' revision (default behavior). prior commit. If no documents have been loaded into the buffer for this collection, this will return an empty response (default) -Samples -^^^^^^^ +Sample +^^^^^^ :: @@ -877,7 +899,6 @@ is an optional parameter. Sample ^^^^^^ - :: $ shipyard logs step/01C9VVQSCFS7V9QB5GBS3WFVSE/action_xcom @@ -928,7 +949,6 @@ Provides topical help for shipyard. Sample ^^^^^^ - :: $ shipyard help diff --git a/doc/source/_static/shipyard.policy.yaml.sample b/doc/source/_static/shipyard.policy.yaml.sample index aebbbcb9..1692ac5b 100644 --- a/doc/source/_static/shipyard.policy.yaml.sample +++ b/doc/source/_static/shipyard.policy.yaml.sample @@ -62,6 +62,11 @@ # GET /api/v1.0/workflows/{id} #"workflow_orchestrator:get_workflow": "rule:admin_required" +# Retrieve the details for a note. Further authorization is required +# depending on the topic of the note +# GET /api/v1.0/notedetails/{note_id} +#"workflow_orchestrator:get_notedetails": "rule:admin_required" + # Retrieve the statuses for the site # GET /api/v1.0/site_statuses #"workflow_orchestrator:get_site_statuses": "rule:admin_required" diff --git a/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample b/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample index aebbbcb9..1692ac5b 100644 --- a/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample +++ b/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample @@ -62,6 +62,11 @@ # GET /api/v1.0/workflows/{id} #"workflow_orchestrator:get_workflow": "rule:admin_required" +# Retrieve the details for a note. Further authorization is required +# depending on the topic of the note +# GET /api/v1.0/notedetails/{note_id} +#"workflow_orchestrator:get_notedetails": "rule:admin_required" + # Retrieve the statuses for the site # GET /api/v1.0/site_statuses #"workflow_orchestrator:get_site_statuses": "rule:admin_required" diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py index d79f2008..7040cc75 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py @@ -42,3 +42,28 @@ class NotesStorageError(NotesError): Raised when there is an error attempting to store a note. """ pass + + +class NoteNotFoundError(NotesRetrievalError): + """NoteNotFoundError + + Raised when a note is looked up directly and not found + """ + pass + + +class NoteURLNotSpecifiedError(NotesRetrievalError): + """NoteURLNotSpecifiedError + + Raised when a note's url info is requested for a note that has no URL + """ + pass + + +class NoteURLRetrievalError(NotesRetrievalError): + """NoteURLRetrievalError + + Raised when there is an error retrieving the URL information for a note + that has a URL specified. + """ + pass diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py index af057d8f..d17620f1 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py @@ -30,6 +30,8 @@ from requests.exceptions import HTTPError from requests.exceptions import RequestException import ulid +from .errors import NoteURLNotSpecifiedError +from .errors import NoteURLRetrievalError from .errors import NotesInitializationError from .errors import NotesRetrievalError from .errors import NotesStorageError @@ -93,8 +95,9 @@ class NotesManager: immediately upon creation, if true """ n = Note(assoc_id, subject, sub_type, note_val, - verbosity=None, link_url=None, is_auth_link=None, - note_id=None, note_timestamp=None) + verbosity=verbosity, link_url=link_url, + is_auth_link=is_auth_link, note_id=note_id, + note_timestamp=note_timestamp) if store: return self.store(n) else: @@ -122,6 +125,8 @@ class NotesManager: """Retrieve a list of notes :param query: a query object to retrieve notes + :returns: a list of notes matchin the query, or [] if there are no + notes matching the query. """ try: notes = list(self.storage.retrieve(query)) @@ -132,45 +137,41 @@ class NotesManager: raise NotesRetrievalError( "Unhandled error during retrieval of notes" ) - # Get the auth token once per retrieve, not once per note. - if notes: - auth_token = self.get_token() - # resolve the note urls - # TODO: threaded? - for note in notes: - self._resolve_note_url(note, auth_token) + for note in notes: + if note.link_url: + note.resolved_url_value = ( + "Details at notedetails/{}".format(note.note_id)) return notes - def _resolve_note_url(self, note, auth_token): - """Resolve and set the value obtained from the URL for a Note. + def retrieve_by_id(self, note_id): + """Return a single note looked up by the specified note_id - :param note: the Note object to retreive and set the value for. - :param auth_token: the authorization token set as a header for the URL - request if one is indicated as needed by the note. - - If there is data retrieved at the note's url, set the - resolved_url_value with those contents. - - If there is no url for the note, return, with resolved_url_value as - None - - If there is no data retrieved, resolved_url_value for the note remains - None - - If there is an error related to retreiving the note's url value, the - resolved_url_value is set to a placeholder value indicating that the - value could not be obtained. + :param note_id: the ULID of the note to retrieve. + :raises NoteNotFoundError: if there is no note matching the requested + note_id """ + return self.storage.retrieve_by_id(note_id) + + def get_note_url_info(self, note): + """Resolve and return the value obtained from the URL for a Note. + + :param note: the note object or id of the note to retreive and set the + value for. + :returns: The contents retrieved from the note's URL. + :raises NoteNotFoundError: when the note (id) specified does not match + a known note + :raises NoteURLNotSpecifiedError: when the note has no url specified + :raises NoteURLRetrievalError: when there is an error using the + note's specified URL. + """ + # if the note is not a note, try to fetch it like an ID if not isinstance(note, Note): - LOG.debug( - "Note is None or not a Note object. URL will not be resolved" - ) - return - + note = self.retrieve_by_id(note) if not note.link_url: - LOG.debug("Note %s has no link to resolve", note.note_id) - return + LOG.debug("Note %s has no URL to resolve.", note.note_id) + raise NoteURLNotSpecifiedError() + auth_token = self.get_token() contents = None try: headers = {} @@ -186,32 +187,11 @@ class NotesManager: response.raise_for_status() # Set the valid response text to the note - note.resolved_url_value = response.text + return response.text - except HTTPError as he: - # A bad status code - don't stop, but log and indicate in note. - LOG.info( - "Note %s has a url returning a bad status code: %s", - note.note_id, response.status_code - ) - note.resolved_url_value = ( - "Note contents could not be retrieved. URL lookup failed " - "with status code: {}" - ).format(response.status_code) - except RequestException as rex: - # A more serious exception; log and indicate in the note - LOG.exception(rex) - note.resolved_url_value = ( - "Note contents could not be retrieved. URL lookup was unable " - "to complete" - ) - except Exception as ex: - # Whatever's left, log and indicate in the note - LOG.exception(ex) - note.resolved_url_value = ( - "Note contents could not be retrieved due to unexpected " - "circumstances" - ) + except (HTTPError, RequestException) as lookup_err: + LOG.exception(lookup_err) + raise NoteURLRetrievalError() class Note: @@ -316,3 +296,15 @@ class NotesStorage(metaclass=abc.ABCMeta): results. """ pass + + @abc.abstractmethod + def retrieve_by_id(self, note_id): + """Lookup a note by note_id + + :param note_id: The ID of the note to retrieve + :returns: a single Note object matching the id or None if there is no + note matching the ID. + :raises NotesRetrievalError: if there is a failure to retrieve the + note + """ + pass 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 6361125b..b8c081a2 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 @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +from enum import Enum import logging from .notes import MAX_VERBOSITY @@ -20,41 +21,105 @@ from .notes import Query LOG = logging.getLogger(__name__) -# Constants and magic numbers for actions: -# [7:33] to slice a string like: -# -# 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 _NoteType: + """Define the patterns and pertinent positions of information for a note + + :param root: the string used as the initial identifier for the type + :param key_pattern_count: the number of variable positions in the key. + E.g.: a value of 3, and a root of "tacos" would generate a key_pattern + of "tacos/{}/{}/{}", while a value of 0 would generate "tacos". + The lookup_pattern for a type is also drived from the key_pattern_count + by subtracting 1 (minimum 0), such that a value of 3 and a root of + "tacos" would generate a lookup_pattern of "tacos/{}/{}/ + :param id_start: the start location in the assoc_id of a note of this type + where the id of the item it is associated with appears. + :param id_end: the optional end location in the assoc_id for locating the + id from the assoc_id of a note of this type. This is only valid for + items that have a fixed length key. + :param default_subtype: the default sub_type specified upon creation of a + note of this type. + """ + + def __init__(self, root, key_pattern_count, id_start, + id_end=None, default_subtype="metadata"): + self.root = root + self.base_pattern = "{}/".format(root) + self.kp_count = key_pattern_count + self.key_pattern = "{}{}".format(root, "/{}" * self.kp_count) + self.lp_count = self.kp_count - 1 if self.kp_count > 0 else 0 + self.lookup_pattern = "{}/{}".format(root, "{}/" * self.lp_count) + self.id_start = id_start + self.id_end = id_end + self.default_subtype = default_subtype + + +class NoteType(Enum): + + # Action + # + # 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 + ACTION = _NoteType( + root="action", + key_pattern_count=1, + id_start=7, + id_end=33, + default_subtype="action metadata") + + # Step + # + # 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 = _NoteType( + root="step", + key_pattern_count=2, + id_start=32, + default_subtype="step metadata") + + OTHER = _NoteType(root="", key_pattern_count=0, id_start=0) + + @property + def base_pattern(self): + return self.value.base_pattern + + @property + def key_pattern(self): + return self.value.key_pattern + + @property + def lookup_pattern(self): + return self.value.lookup_pattern + + @property + def id_start(self): + return self.value.id_start + + @property + def id_end(self): + return self.value.id_end + + @property + def default_subtype(self): + return self.value.default_subtype + + @classmethod + def get_type(cls, note): + for tp in cls: + if note.assoc_id.startswith(tp.value.base_pattern): + return tp + return cls.OTHER class NotesHelper: @@ -68,7 +133,7 @@ class NotesHelper: def _failsafe_make_note(self, assoc_id, subject, sub_type, note_val, verbosity=MIN_VERBOSITY, link_url=None, - is_auth_link=None): + is_auth_link=None, note_timestamp=None): """LOG and continue on any note creation failure""" try: self.nm.create( @@ -79,6 +144,7 @@ class NotesHelper: verbosity=verbosity, link_url=link_url, is_auth_link=is_auth_link, + note_timestamp=note_timestamp ) except Exception as ex: LOG.warn( @@ -105,13 +171,45 @@ class NotesHelper: LOG.exception(ex) return [] + # + # Retrieve notes by note ID + # + + def get_note(self, note_id): + """Return a single note looked up by the specified note_id + + :param note_id: the ULID of the note to retrieve. + :raises NoteNotFoundError: if there is no note matching the requested + note_id + """ + return self.nm.retrieve_by_id(note_id) + + def get_note_details(self, note): + """Return the note details for the specified note + + :param note: the Note object with additional details to retrieve. + """ + return self.nm.get_note_url_info(note) + + def get_note_assoc_id_type(self, note): + """Return the type of note based on the assoc_id + + :param note: The note to examine + + The purpose of this method is to use the standard formats (e.g.: + action and step) supported by this helper to get the type of note. + This value can be used by a client to enforce access rules to the note + based on the item it is related to. + """ + return NoteType.get_type(note) + # # Action notes helper methods # def make_action_note(self, action_id, note_val, subject=None, sub_type=None, verbosity=MIN_VERBOSITY, link_url=None, - is_auth_link=None): + is_auth_link=None, note_timestamp=None): """Creates an action note using a convention for the note's assoc_id :param action_id: the ULID id of an action @@ -127,12 +225,14 @@ class NotesHelper: :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 + :param note_timestamp: the parseable string timestamp to associate with + this note. Optional, defaults to the creation time of the note. """ - assoc_id = ACTION_KEY_PATTERN.format(action_id) + assoc_id = NoteType.ACTION.key_pattern.format(action_id) if subject is None: subject = action_id if sub_type is None: - sub_type = "action metadata" + sub_type = NoteType.ACTION.default_subtype self._failsafe_make_note( assoc_id=assoc_id, @@ -142,6 +242,7 @@ class NotesHelper: verbosity=verbosity, link_url=link_url, is_auth_link=is_auth_link, + note_timestamp=note_timestamp ) def get_all_action_notes(self, verbosity=MIN_VERBOSITY): @@ -150,19 +251,17 @@ class NotesHelper: :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. """ notes = self._failsafe_get_notes( - assoc_id_pattern=ACTION_LOOKUP_PATTERN, + assoc_id_pattern=NoteType.ACTION.lookup_pattern, verbosity=verbosity, exact_match=False ) note_dict = {} + id_s = NoteType.ACTION.id_start + id_e = NoteType.ACTION.id_end for n in notes: - action_id = n.assoc_id[ACTION_ID_START:ACTION_ID_END] + action_id = n.assoc_id[id_s:id_e] if action_id not in note_dict: note_dict[action_id] = [] note_dict[action_id].append(n) @@ -178,7 +277,7 @@ class NotesHelper: """ return self._failsafe_get_notes( - assoc_id_pattern=ACTION_KEY_PATTERN.format(action_id), + assoc_id_pattern=NoteType.ACTION.key_pattern.format(action_id), verbosity=verbosity, exact_match=True ) @@ -189,7 +288,7 @@ class NotesHelper: 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): + is_auth_link=None, note_timestamp=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 @@ -206,12 +305,14 @@ class NotesHelper: :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 + :param note_timestamp: the parseable string timestamp to associate with + this note. Optional, defaults to the creation time of the note. """ - assoc_id = STEP_KEY_PATTERN.format(action_id, step_id) + assoc_id = NoteType.STEP.key_pattern.format(action_id, step_id) if subject is None: subject = step_id if sub_type is None: - sub_type = "step metadata" + sub_type = NoteType.STEP.default_subtype self._failsafe_make_note( assoc_id=assoc_id, @@ -221,6 +322,7 @@ class NotesHelper: verbosity=verbosity, link_url=link_url, is_auth_link=is_auth_link, + note_timestamp=note_timestamp ) def get_all_step_notes_for_action(self, action_id, @@ -233,13 +335,14 @@ class NotesHelper: if set to less than 1, returns {}, skipping any retrieval """ notes = self._failsafe_get_notes( - assoc_id_pattern=STEP_LOOKUP_PATTERN.format(action_id), + assoc_id_pattern=NoteType.STEP.lookup_pattern.format(action_id), verbosity=verbosity, exact_match=False ) note_dict = {} + id_s = NoteType.STEP.id_start for n in notes: - step_id = n.assoc_id[STEP_ID_START:] + step_id = n.assoc_id[id_s:] if step_id not in note_dict: note_dict[step_id] = [] note_dict[step_id].append(n) @@ -256,7 +359,8 @@ class NotesHelper: """ return self._failsafe_get_notes( - assoc_id_pattern=STEP_KEY_PATTERN.format(action_id, step_id), + assoc_id_pattern=NoteType.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 ead5a70f..9e96fc94 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 @@ -34,6 +34,7 @@ from sqlalchemy.orm import sessionmaker from .notes import Note from .notes import NotesStorage +from .errors import NoteNotFoundError from .errors import NotesError from .errors import NotesInitializationError @@ -128,19 +129,27 @@ class ShipyardSQLNotesStorage(NotesStorage): TNote.assoc_id == a_id_pat, TNote.verbosity <= max_verb ) - ) + ).order_by(TNote.note_timestamp) else: n_qry = session.query(TNote).filter( and_( TNote.assoc_id.like(a_id_pat + '%'), TNote.verbosity <= max_verb ) - ) + ).order_by(TNote.note_timestamp) db_notes = n_qry.all() for tn in db_notes: r_notes.append(self._map(tn, Note)) return r_notes + def retrieve_by_id(self, note_id): + with self.session_scope() as session: + note = session.query(TNote).filter( + TNote.note_id == note_id).one_or_none() + if not note: + raise NoteNotFoundError() + return self._map(note, Note) + def _map(self, src, target_type): """Maps a Note object to/from a TNote object. 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 76b3aab6..2c83f846 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 @@ -13,6 +13,7 @@ # limitations under the License. # """Implementations of the NotesStorage base class""" +from .errors import NoteNotFoundError from .notes import NotesStorage @@ -41,4 +42,11 @@ class MemoryNotesStorage(NotesStorage): if (note.assoc_id.startswith(pat) and note.verbosity <= max_verb): notes.append(note) + notes.sort(key=lambda x: x.note_timestamp) return notes + + def retrieve_by_id(self, note_id): + note = self.storage.get(note_id) + if not note: + raise NoteNotFoundError() + return note diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/api.py index 22b0893b..c92af916 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/api.py @@ -42,6 +42,7 @@ 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.notes.notedetails_api import NoteDetailsResource from shipyard_airflow.control.status.status_api import StatusResource from shipyard_airflow.errors import (AppError, default_error_serializer, default_exception_handler) @@ -78,6 +79,7 @@ def start_api(): ('/configdocs', ConfigDocsStatusResource()), ('/configdocs/{collection_id}', ConfigDocsResource()), ('/commitconfigdocs', CommitConfigDocsResource()), + ('/notedetails/{note_id}', NoteDetailsResource()), ('/renderedconfigdocs', RenderedConfigDocsResource()), ('/workflows', WorkflowResource()), ('/workflows/{workflow_id}', WorkflowIdResource()), diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/notes/__init__.py b/src/bin/shipyard_airflow/shipyard_airflow/control/notes/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/notes/notedetails_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/notes/notedetails_api.py new file mode 100644 index 00000000..ece1ce80 --- /dev/null +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/notes/notedetails_api.py @@ -0,0 +1,105 @@ +# 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. +import falcon + +from shipyard_airflow.common.notes.errors import NoteNotFoundError +from shipyard_airflow.common.notes.errors import NoteURLNotSpecifiedError +from shipyard_airflow.common.notes.errors import NoteURLRetrievalError +from shipyard_airflow.common.notes.notes_helper import NoteType +from shipyard_airflow.control.base import BaseResource +from shipyard_airflow.control.helpers.notes import NOTES as notes_helper +from shipyard_airflow.errors import ApiError +from shipyard_airflow.errors import InvalidFormatError +from shipyard_airflow import policy + + +NOTE_TYPE_RBAC = { + NoteType.ACTION: policy.GET_ACTION, + NoteType.STEP: policy.GET_ACTION_STEP, + # Anything else uses only the already checked GET_NOTEDETAILS + # new known note types should be added to the notes helper and also + # represented here. + NoteType.OTHER: None +} + + +# /api/v1.0/notedetails/{note_id} +class NoteDetailsResource(BaseResource): + """Resource to service requests for note details""" + @policy.ApiEnforcer(policy.GET_NOTEDETAILS) + def on_get(self, req, resp, **kwargs): + """Retrieves additional information for a note. + + Using the specified note_id, looks up any additional information + for a note + """ + note_id = kwargs['note_id'] + self.validate_note_id(note_id) + note = self.get_note_with_access_check(req.context, note_id) + resp.body = self.get_note_details(note) + resp.status = falcon.HTTP_200 + + def validate_note_id(self, note_id): + if not len(note_id) == 26: + raise InvalidFormatError( + title="Notes ID values are 26 character ULID values", + description="Invalid note_id: {} in URL".format(note_id) + ) + + def get_note_with_access_check(self, context, note_id): + """Retrieve the note and checks user access to the note + + :param context: the request context + :param note_id: the id of the note to retrieve. + :returns: the note + """ + try: + note = notes_helper.get_note(note_id) + note_type = notes_helper.get_note_assoc_id_type(note) + if note_type not in NOTE_TYPE_RBAC: + raise ApiError( + title="Unable to check permission for note type", + description=( + "Shipyard is not correctly identifying note type " + "for note {}".format(note_id)), + status=falcon.HTTP_500, + retry=False) + policy.check_auth(context, NOTE_TYPE_RBAC[note_type]) + return note + except NoteNotFoundError: + raise ApiError( + title="No note found", + description=("Note {} is not found".format(note_id)), + status=falcon.HTTP_404) + + def get_note_details(self, note): + """Retrieve the note details from the notes_helper + + :param note: the note with extended information + """ + try: + return notes_helper.get_note_details(note) + except NoteURLNotSpecifiedError: + raise ApiError( + title="No further note details are available", + description=("Note {} has no additional information to " + "return".format(note.note_id)), + status=falcon.HTTP_404) + except NoteURLRetrievalError: + raise ApiError( + title="Unable to retrieve URL information for note", + description=("Note {} has additional information, but it " + "cannot be accessed by Shipyard at this " + "time".format(note.note_id)), + status=falcon.HTTP_500) diff --git a/src/bin/shipyard_airflow/shipyard_airflow/policy.py b/src/bin/shipyard_airflow/shipyard_airflow/policy.py index 5e10aae8..731503c2 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/policy.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/policy.py @@ -40,6 +40,7 @@ COMMIT_CONFIGDOCS = 'workflow_orchestrator:commit_configdocs' GET_RENDEREDCONFIGDOCS = 'workflow_orchestrator:get_renderedconfigdocs' LIST_WORKFLOWS = 'workflow_orchestrator:list_workflows' GET_WORKFLOW = 'workflow_orchestrator:get_workflow' +GET_NOTEDETAILS = 'workflow_orchestrator:get_notedetails' GET_SITE_STATUSES = 'workflow_orchestrator:get_site_statuses' ACTION_DEPLOY_SITE = 'workflow_orchestrator:action_deploy_site' ACTION_UPDATE_SITE = 'workflow_orchestrator:action_update_site' @@ -206,6 +207,16 @@ class ShipyardPolicy(object): 'method': 'GET' }] ), + policy.DocumentedRuleDefault( + GET_NOTEDETAILS, + RULE_ADMIN_REQUIRED, + ('Retrieve the details for a note. Further authorization is ' + 'required depending on the topic of the note'), + [{ + 'path': '/api/v1.0/notedetails/{note_id}', + 'method': 'GET' + }] + ), policy.DocumentedRuleDefault( GET_SITE_STATUSES, RULE_ADMIN_REQUIRED, diff --git a/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py b/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py index 397bd117..1a70656b 100644 --- a/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py +++ b/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py @@ -20,7 +20,10 @@ import responses from shipyard_airflow.common.notes.errors import ( NotesInitializationError, NotesRetrievalError, - NotesStorageError + NotesStorageError, + NoteNotFoundError, + NoteURLNotSpecifiedError, + NoteURLRetrievalError ) from shipyard_airflow.common.notes.notes import ( Note, @@ -42,6 +45,9 @@ class NotesStorageErrorImpl(NotesStorage): def retrieve(self, query): raise Exception("Outta Nowhere") + def retrieve_by_id(self, note_id): + raise Exception("No warning") + class NotesStorageExpectedErrorImpl(NotesStorage): def store(self, note): @@ -50,6 +56,9 @@ class NotesStorageExpectedErrorImpl(NotesStorage): def retrieve(self, query): raise NotesRetrievalError("Expected") + def retrieve_by_id(self, note_id): + raise NotesRetrievalError("Expected") + class TestNotesManager: def test_init(self): @@ -191,23 +200,8 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1/11111/aaa", exact_match=True)) assert len(n_list) == 1 - @responses.activate - def test_store_retrieve_urls(self): - responses.add( - method="GET", - url="http://test.test", - body="Hello from testland", - status=200, - content_type="text/plain" - ) - responses.add( - method="GET", - url="http://test.test2", - body="Hello from testland2", - status=200, - content_type="text/plain" - ) - + def test_store_retrieve_url_refs(self): + """Tests that notes retrieved as a list have notedetails refs""" nm = NotesManager(MemoryNotesStorage(), get_token) nm.store(Note( assoc_id="test1/11111/aaa", @@ -226,7 +220,46 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1")) assert len(n_list) == 2 for n in n_list: - assert n.resolved_url_value.startswith("Hello from testland") + assert n.resolved_url_value == ( + "Details at notedetails/" + n.note_id) + + @responses.activate + def test_store_retrieve_urls(self): + responses.add( + method="GET", + url="http://test.test", + body="Hello from testland", + status=200, + content_type="text/plain" + ) + responses.add( + method="GET", + url="http://test.test2", + body="Hello from testland2", + status=200, + content_type="text/plain" + ) + nm = NotesManager(MemoryNotesStorage(), get_token) + nm.store(Note( + assoc_id="test1/11111/aaa", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 1", + link_url="http://test.test/" + )) + nm.store(Note( + assoc_id="test1/11111/bbb", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 2", + link_url="http://test.test2/" + )) + n_list = nm.retrieve(Query("test1")) + assert len(n_list) == 2 + for n in n_list: + assert n.resolved_url_value.startswith("Details at notedetails/") + assert nm.get_note_url_info(n.note_id).startswith( + "Hello from testland") with pytest.raises(KeyError): auth_hdr = responses.calls[0].request.headers['X-Auth-Token'] @@ -265,10 +298,14 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1")) assert len(n_list) == 2 for n in n_list: + assert n.resolved_url_value == ( + "Details at notedetails/" + n.note_id) if n.assoc_id == "test1/11111/aaa": - assert "failed with status code: 404" in n.resolved_url_value + with pytest.raises(NoteURLRetrievalError): + nd = nm.get_note_url_info(n.note_id) else: - assert n.resolved_url_value.startswith("Hello from testland") + nd = nm.get_note_url_info(n.note_id) + assert nd.startswith("Hello from testland") @responses.activate def test_store_retrieve_url_does_not_exist(self): @@ -299,9 +336,33 @@ class TestNotesManager: assert len(n_list) == 2 for n in n_list: if n.assoc_id == "test1/11111/aaa": - assert "URL lookup was unable" in n.resolved_url_value + with pytest.raises(NoteURLRetrievalError): + nd = nm.get_note_url_info(n.note_id) else: - assert n.resolved_url_value.startswith("Hello from testland") + nd = nm.get_note_url_info(n.note_id) + assert nd.startswith("Hello from testland") + + def test_store_retrieve_url_not_specified(self): + nm = NotesManager(MemoryNotesStorage(), get_token) + nm.store(Note( + assoc_id="test1/11111/aaa", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 1", + link_url="" + )) + nm.store(Note( + assoc_id="test1/11111/bbb", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 2", + link_url=None + )) + n_list = nm.retrieve(Query("test1")) + assert len(n_list) == 2 + for n in n_list: + with pytest.raises(NoteURLNotSpecifiedError): + nd = nm.get_note_url_info(n.note_id) @responses.activate def test_store_retrieve_with_auth(self): @@ -325,7 +386,8 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1")) assert len(n_list) == 1 for n in n_list: - assert n.resolved_url_value == "Hello from testland2" + nd = nm.get_note_url_info(n.note_id) + assert nd == "Hello from testland2" auth_hdr = responses.calls[0].request.headers['X-Auth-Token'] assert 'token' == auth_hdr diff --git a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py index cf1590c5..d91b06c6 100644 --- a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py +++ b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py @@ -34,6 +34,7 @@ class ApiPaths(enum.Enum): GET_STEP_DETAIL = _BASE_URL + 'actions/{}/steps/{}' GET_STEP_LOG = _BASE_URL + 'actions/{}/steps/{}/logs' POST_CONTROL_ACTION = _BASE_URL + 'actions/{}/control/{}' + GET_NOTEDETAILS = _BASE_URL + 'notedetails/{}' GET_WORKFLOWS = _BASE_URL + 'workflows' GET_DAG_DETAIL = _BASE_URL + 'workflows/{}' GET_SITE_STATUSES = _BASE_URL + 'site_statuses' @@ -225,6 +226,15 @@ class ShipyardClient(BaseClient): self.get_endpoint(), action_id, control_verb) return self.post_resp(url) + def get_note_details(self, note_id): + """Retrieve note details from a note's associated information + + :param note_id: The ID of the note having additional information + """ + url = ApiPaths.GET_NOTEDETAILS.value.format( + self.get_endpoint(), note_id) + return self.get_resp(url) + def get_workflows(self, since=None): """ Queries airflow for DAGs that are running or have run 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 983edbf5..bc5ee9b4 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 @@ -25,7 +25,7 @@ def gen_action_steps(step_list, action_id): """ # Generate the steps table. steps = format_utils.table_factory( - field_names=['Steps', 'Index', 'State', 'Notes'] + field_names=['Steps', 'Index', 'State', 'Footnotes'] ) # rendered notes , a list of lists of notes r_notes = [] @@ -34,7 +34,7 @@ def gen_action_steps(step_list, action_id): for step in step_list: notes = step.get('notes') if notes: - r_notes.append(format_utils.format_notes(notes)) + r_notes.append(format_notes(notes)) steps.add_row([ 'step/{}/{}'.format(action_id, step.get('id')), step.get('index'), @@ -44,16 +44,9 @@ def gen_action_steps(step_list, action_id): else: steps.add_row(['None', '', '', '']) - 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 + return "{}\n\n{}".format( + format_utils.table_get_string(steps), + notes_table("Step", r_notes)) def gen_action_commands(command_list): @@ -141,7 +134,7 @@ def gen_action_table(action_list): """ actions = format_utils.table_factory( field_names=['Name', 'Action', 'Lifecycle', 'Execution Time', - 'Step Succ/Fail/Oth', 'Notes']) + 'Step Succ/Fail/Oth', 'Footnotes']) # list of lists of rendered notes r_notes = [] if action_list: @@ -149,7 +142,7 @@ def gen_action_table(action_list): 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)) + r_notes.append(format_notes(notes)) actions.add_row([ action.get('name'), 'action/{}'.format(action.get('id')), @@ -161,16 +154,9 @@ def gen_action_table(action_list): else: actions.add_row(['None', '', '', '', '', '']) - 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 + return "{}\n\n{}".format( + format_utils.table_get_string(actions), + notes_table("Action", r_notes)) def _step_summary(step_list): @@ -370,13 +356,88 @@ def _site_statuses_switcher(status_type): return call_func -def gen_detail_notes(dict_with_notes): + +def gen_detail_notes(title, dict_with_notes): """Generates a standard formatted section of notes + :param title: the title for the notes section. E.g.: "Step" :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', [])) + n_strings = format_notes(dict_with_notes.get('notes', [])) if n_strings: - return "Notes:\n{}".format("\n".join(n_strings)) + return "{} Notes:\n{}".format(title, "\n".join(n_strings)) return "" + + +def notes_table(title, notes_list): + """Format a table of notes + + :param title: the header for the table. e.g.: "Step" + :param list notes_list: a list of lists of formatted notes: + e.g.:[[note1,note2],[note3]] + The notes ideally have been pre-formatted by "format_notes" + + :returns: string of a table e.g.: + + Step Notes Note + (1) > note1 + - Info avail... + > note2 + (2) > note3 + + If notes_list is empty, returns an empty string. + """ + if not notes_list: + return "" + headers = ["{} Footnotes".format(title), "Note"] + rows = [] + index = 1 + for notes in notes_list: + rows.append(["({})".format(index), "\n".join(notes)]) + index += 1 + return format_utils.table_get_string( + format_utils.table_factory(headers, rows)) + + +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': "message", + 'verbosity': 1, + 'note_id': "09876543210987654321098765", + 'note_timestamp': "2018-10-08 14:23:53.346534", + 'resolved_url_value': \ + "Details at notedetails/09876543210987654321098765" + } + + Resulting in: + + > action:12345678901234567890123456(2018-10-08 14:23:53.346534): message + - Info available with 'describe notedetails/09876543210987654321098765' + """ + 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 - Info available with " + "'describe notedetails/{}'".format(n['note_id'])) + except KeyError: + s = "!!! Unparseable Note: {}".format(n) + + nl.append(s) + return nl 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 44bc4eae..0f8a3f5a 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py +++ b/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py @@ -53,10 +53,48 @@ class DescribeAction(CliAction): cli_format_common.gen_action_validations( resp_j.get('validations') ), - cli_format_common.gen_detail_notes(resp_j) + cli_format_common.gen_detail_notes("Action", resp_j) ) +class DescribeNotedetails(CliAction): + """Action to Describe notedetails""" + + def __init__(self, ctx, note_id): + """Sets parameters.""" + super().__init__(ctx) + self.logger.debug( + "DescribeNotedetails action initialized with note_id=%s", note_id) + self.note_id = note_id + + def invoke(self): + """Calls API Client and formats response from API Client""" + self.logger.debug("Calling API Client get_note_details.") + return self.get_api_client().get_note_details( + note_id=self.note_id) + + # Handle 404 with default error handler for cli. + cli_handled_err_resp_codes = [400, 404, 500] + + # Handle 200 responses using the cli_format_response_handler + cli_handled_succ_resp_codes = [200] + + def cli_format_response_handler(self, response): + """CLI output handler + + :param response: a requests response object containing the details + :returns: a string representing a formatted response + + Handles 200 responses. If the response contains '\n' characters + (literally), this will attempt to replace with newline characters + """ + resp = response.text + if "\\n" in resp: + return "\n".join(resp.split("\\n")) + else: + return resp + + class DescribeStep(CliAction): """Action to Describe Step""" @@ -91,7 +129,7 @@ class DescribeStep(CliAction): resp_j = response.json() 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) + cli_format_common.gen_detail_notes("Step", resp_j) ) diff --git a/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py b/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py index d4d8864a..9da41ef5 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py @@ -19,6 +19,7 @@ import click from click_default_group import DefaultGroup from shipyard_client.cli.describe.actions import DescribeAction +from shipyard_client.cli.describe.actions import DescribeNotedetails from shipyard_client.cli.describe.actions import DescribeStep from shipyard_client.cli.describe.actions import DescribeValidation from shipyard_client.cli.describe.actions import DescribeWorkflow @@ -29,7 +30,7 @@ from shipyard_client.cli.input_checks import check_id, check_workflow_id @click.pass_context def describe(ctx): """ - Describe the action, step, or validation. \n + Describe the action, step, note details or validation. \n For more information on describe commands please enter the describe command followed by '--help' \n Example: shipyard describe action --help \n @@ -41,7 +42,8 @@ def describe(ctx): FORMAT: shipyard describe \n EXAMPLE: shipyard describe action/01BTG32JW87G0YKA1K29TKNAFX | shipyard describe step/01BTG32JW87G0YKA1K29TKNAFX/preflight | shipyard describe - validation/01BTG32JW87G0YKA1K29TKNAFX/01BTG3PKBS15KCKFZ56XXXBGF2 + validation/01BTG32JW87G0YKA1K29TKNAFX/01BTG3PKBS15KCKFZ56XXXBGF2 | shipyard + describe notedetails/01BTG32JW87G0YKA1KA9AKAAB3 """ @@ -64,8 +66,11 @@ def describe_default_command(ctx, namespace_item): elif namespace[0] == 'workflow': ctx.invoke( describe_workflow, - workflow_id=namespace[1] - ) + workflow_id=namespace[1]) + elif namespace[0] == 'notedetails': + ctx.invoke( + describe_notedetails, + note_id=namespace[1]) else: raise Exception('Invalid namespaced describe action') except Exception: @@ -74,7 +79,8 @@ def describe_default_command(ctx, namespace_item): "action: action/action id\n" "step: step/action id/step id\n" "validation: validation/validation id/action id\n" - "workflow: workflow/workflow id") + "workflow: workflow/workflow id\n" + "notedetails: notedetails/note_id") DESC_ACTION = """ @@ -102,6 +108,32 @@ def describe_action(ctx, action_id): click.echo(DescribeAction(ctx, action_id).invoke_and_return_resp()) +DESC_NOTEDETAILS = """ +COMMAND: describe notedetials \n +DESCRIPTION: Retrieves the detailed information that is associated with the +specified note id. \n +FORMAT: shipyard describe notedetails \n +EXAMPLE: shipyard describe notedetails 01BTG32JW87G0YKA1KA9AKAAB3 +""" + +SHORT_DESC_NOTEDETAILS = ( + "Retrieves the detailed information about the supplied action id.") + + +@describe.command('notedetails', + help=DESC_NOTEDETAILS, short_help=SHORT_DESC_NOTEDETAILS) +@click.argument('note_id') +@click.pass_context +def describe_notedetails(ctx, note_id): + + if not note_id: + ctx.fail("A note id argument must be passed.") + + check_id(ctx, note_id) + + click.echo(DescribeNotedetails(ctx, note_id).invoke_and_return_resp()) + + DESC_STEP = """ COMMAND: describe step \n DESCRIPTION: Retrieves the step details associated with an action and step. \n 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 2f6cc6e8..5ab1c9c6 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/format_utils.py +++ b/src/bin/shipyard_client/shipyard_client/cli/format_utils.py @@ -234,41 +234,3 @@ 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/shipyard_client/cli/input_checks.py b/src/bin/shipyard_client/shipyard_client/cli/input_checks.py index 61f254d3..acef121e 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/input_checks.py +++ b/src/bin/shipyard_client/shipyard_client/cli/input_checks.py @@ -31,13 +31,13 @@ def check_control_action(ctx, action): ctx.fail('Invalid action. Please enter pause, unpause, or stop.') -def check_id(ctx, action_id): +def check_id(ctx, ulid_id): """Verifies a ULID id is in a valid format""" - if action_id is None: + if ulid_id is None: ctx.fail('Invalid ID. None is not a valid action ID.') - if len(action_id) != 26: + if len(ulid_id) != 26: ctx.fail('Invalid ID. ID can only be 26 characters.') - if not action_id.isalnum(): + if not ulid_id.isalnum(): ctx.fail('Invalid ID. ID can only contain letters and numbers.') diff --git a/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py b/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py index 3104c403..3198a4cd 100644 --- a/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py +++ b/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py @@ -196,6 +196,17 @@ def test_post_control(*args): shipyard_client.get_endpoint(), action_id, control_verb) +@mock.patch.object(BaseClient, 'post_resp', replace_post_rep) +@mock.patch.object(BaseClient, 'get_resp', replace_get_resp) +@mock.patch.object(BaseClient, 'get_endpoint', replace_get_endpoint) +def test_get_note_details(*args): + shipyard_client = get_api_client() + note_id = "ABC123ABC123ZZABC123ABC123" + result = shipyard_client.get_note_details(note_id) + assert result['url'] == '{}/notedetails/{}'.format( + shipyard_client.get_endpoint(), note_id) + + @mock.patch.object(BaseClient, 'post_resp', replace_post_rep) @mock.patch.object(BaseClient, 'get_resp', replace_get_resp) @mock.patch.object(BaseClient, 'get_endpoint', replace_get_endpoint) 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 105c35a3..12ffb0e9 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 @@ -104,7 +104,9 @@ def test_get_actions(*args): 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 + assert "Action Footnotes" in response + assert (" - Info available with 'describe notedetails/" + "ABCDEFGHIJKLMNOPQRSTUVWXYA'") in response GET_ACTIONS_API_RESP_UNPARSEABLE_NOTE = """