[GitHub] rawlinp commented on a change in pull request #3266: Added an implementation of the to-access functions to Python client package

[GitBox <gi...@apache.org>]

rawlinp commented on a change in pull request #3266: Added an implementation of the to-access functions to Python client package
URL: https://github.com/apache/trafficcontrol/pull/3266#discussion_r253225572
 
 

 ##########
 File path: traffic_control/clients/python/to_access/__init__.py
 ##########
 @@ -0,0 +1,576 @@
+#
+# 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.
+#
+
+"""
+.. _toaccess:
+
+.. program:: toaccess
+
+``toaccess``
+============
+This module provides a set of functions meant to provide ease-of-use functionality for interacting
+with the Traffic Ops API. It provides scripts named :file:`to{method}` where `method` is the name of
+an HTTP method (in lowercase). Collectively they are referred to as :program:`toaccess` Implemented
+methods thus far are:
+
+- delete
+- head
+- get
+- options
+- patch
+- post
+- put
+
+Arguments and Flags
+-------------------
+.. option:: PATH
+
+	This is the request path. By default, whatever is passed is considered to be relative to
+	:file:`/api/{api_version}/` where ``api_version`` is :option:`--api_version`. This behavior can
+	be disabled by using :option:`--raw_path`.
+
+.. option:: DATA
+
+	An optional positional argument that is a data payload to pass to the Traffic Ops server in the
+	request body. If this is the absolute or relative path to a file, the contents of the file will
+	instead be read and used as the request payload.
+
+.. option:: -h, --help
+
+	Print usage information and exit
+
+.. option:: -a API_VERSION, --api_version API_VERSION
+
+	Specifies the version of the Traffic Ops API that will be used for the request. Has no effect if
+	:option:`--raw_path` is used. (Default: 1.3)
+
+.. option:: -f, --full
+
+	Output the full HTTP exchange including request method line, request headers, request body (if
+	any), response status line, and response headers (as well as the response body, if any). This is
+	equivalent to using :option:`--request_headers`, :option:`--request_payload`, and
+	:option:`--response_headers` at the same time, and those options will have no effect if given.
+	(Default: false)
+
+.. option:: -k, --insecure
+
+	Do not verify SSL certificates - typically useful for making requests to development or testing
+	servers as they frequently have self-signed certificates. (Default: false)
+
+.. option:: -p, --pretty
+
+	Pretty-print any payloads that are output as formatted JSON. Has no effect on plaintext payloads.
+	Uses tab characters for indentation. (Default: false)
+
+.. option:: -r, --raw_path
+
+	Request exactly :option:`PATH`; do not preface the request path with :file:`/api/{api_version}`.
+	This effectively means that :option:`--api_version` will have no effect. (Default: false)
+
+.. option:: --request_headers
+
+	Output the request method line and any and all request headers. (Default: false)
+
+.. option:: --request_payload
+
+	Output the request body if any was sent. Will attempt to pretty-print the body as JSON if
+	:option:`--pretty` is used. (Default: false)
+
+.. option:: --response_headers
+
+	Output the response status line and any and all response headers. (Default: false)
+
+.. option:: --to_host HOST
+
+	The :abbr:`FQDN (Fully Qualified Domain Name)` and optionally the port and scheme of the Traffic
+	Ops server. This will override :envvar:`TO_URL`. The format is the same as for :envvar:`TO_URL`.
+	(Default: uses the value of :envvar:`TO_URL`)
+
+.. option:: --to_password PASSWORD
+
+	The password to use when authenticating to Traffic Ops. Overrides :envvar:`TO_PASSWORD`.
+	(Default: uses the value of :envvar:`TO_PASSWORD`)
+
+.. option:: --to_user USERNAME
+
+	The username to use when connecting to Traffic Ops. Overrides :envvar:`TO_USER`. (Default: uses
+	the value of :envvar:`TO_USER`)
+
+Environment Variables
+---------------------
+If defined, :program:`toaccess` scripts will use these environment variables to define their
+connection to and authentication with the Traffic Ops server. Typically, setting these is easier
+than using the long options :option:`--to_host`, :option:`--to_user`, and :option:`--to_password` on
+every invocation.
+
+.. envvar:: TO_PASSWORD
+
+	Will be used to authenticate the user defined by either :option:`--to_user` or :envvar:`TO_USER`.
+
+.. envvar:: TO_URL
+
+	The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server to which the script
+	will connect. The format of this should be :file:`[{http or https}://]{hostname}[:{port}]`. Note
+	that this may optionally start with ``http://`` or ``https://`` (case insensitive), but
+	typically this is unnecessary. Also notice that the port number may be specified, though again
+	this isn't usually required. All :program:`toaccess` scripts will assume that port 443 should be
+	used unless otherwise specified. They will further assume that the protocol is HTTPS unless
+	:envvar:`TO_URL` (or :option:`--to_host`) starts with ``http://``, in which case the default port
+	will also be set to 80 unless otherwise specified in the URL.
+
+.. envvar:: TO_USER
+
+	The name of the user as whom to connect to the Traffic Ops server.
+
+Module Reference
+================
+
+"""
+import json
+import logging
+import os
+import sys
+import typing
+
+import requests
+
+from trafficops.restapi import LoginError, OperationError, InvalidJSONError
+from trafficops.tosession import TOSession
+
+l = logging.getLogger()
+l.disabled = True
+logging.basicConfig(level=logging.CRITICAL+1)
+
+#: The full path to a file used to store the user's Mojolicious athentication cookie (currently unused)
+COOKIEFILE = "" #os.path.expanduser(os.path.join("~", ".to-auth.cookie"))
+
+def setCookie(cookie:str):
+	"""
+	Writes the passed cookie to the :data:`COOKIEFILE` file for later use.
+
+	.. warning::
+		Currently this function is never used, and it depends on the value of :data:`COOKIEFILE`,
+		which at the moment is being set to an empty string.
+	"""
+	with open(COOKIEFILE, "w") as f:
+		f.write(cookie)
+
+def output(r:requests.Response, pretty:bool, reqHeader:bool, respHeader:bool, reqPayload:bool,
+           indent:typing.Union[int, str] = '\t'):
 
 Review comment:
   https://stackoverflow.blog/2017/06/15/developers-use-spaces-make-money-use-tabs/ -- nuff said.
   
   In all seriousness though, I mainly just prefer spaces in Python because it's the standard due to PEP8 (even though I don't agree everything in it should be followed religiously). You will see it in virtually every other big open source Python project. It's not done because it's better than using tabs, it's done because it keeps the broader community consistent. If you're a Python dev and have your editor set to 4-space indentations, in most cases you wouldn't have to change your editor settings when switching between projects. Depending on the editor, this isn't really a big deal anyways, since editors should be able to determine what type of indentation is being used in a file and adapt accordingly.
   
   I think it would be fine to stick to tabs consistently in our Python code. What worries me is mixing tabs and spaces in a file (e.g. for indentation vs alignment), because I think that can get messy very easily and would be better to just avoid that class of bug altogether by sticking to one or the other. I don't want to get bit by that bug, but if we can reasonably depend on the linter to catch those kinds of things at PR review time, I think I wouldn't mind allowing tabs and spaces on the same line for indentation and alignment, respectively.
   
   However, I strongly believe we should try to stick to _at least_ the PEP8-defined naming schemes in all of the project's Python code. Go and Java code shouldn't use snake_case, just like C/C++ and Python code shouldn't use camelCase. Standard language naming schemes should always be followed IMO.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services