Skip to content

Instantly share code, notes, and snippets.

@abadger
Last active October 17, 2018 17:13
Show Gist options
  • Star 4 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save abadger/3edc108f5a5b88c1a9a46c4869d778fd to your computer and use it in GitHub Desktop.
Save abadger/3edc108f5a5b88c1a9a46c4869d778fd to your computer and use it in GitHub Desktop.
categorization of basic.rst

For the glossary:

distribution

In some operating systems, the kernel of the OS is produced by one organization but this is combined with other necessary tools by a second organization. This second organization is called a distribution.

native string

Python2 and Python3 have different types for unadorned string literals and many strings operations. In Python2, these are byte strings. In Python3, these are text strings.

Note

In very old code, modules would specify that a parameter was a boolean by listing something like this in the argument_spec:

module = AnsibleModule(
argument_spec=dict(

return_status=dict(choices=BOOLEANS),

),

)

This pattern is not very flexible and can cause issues in cornercases. It should no longer be used. Switch to using the type field instead:

module = AnsibleModule(
    argument_spec=dict(
        return_status=dict(type='bool'),
    ),
)

Core team members, should we change get_all_subclasses() to return a set/frozenset? Right now, it returns a list and elements could be repeated

Core team members, do we want to change this API? Instead of returning an instantiated class, return the class and make the caller instantiate it. Doing that will mean we don't have to pass in *args and **kwargs.

Internals

These globals are not for use by other code. Documenting here for people who need to modify basic.py.

Deprecated

All Python2 and Python3 compat

Analogs to these are either in six or one of the ansible.module_utils.pycompatXY modules:

Deprecated New Way to Achieve this ---------- -----------------------imap ansible.module_utils.six.moves.map basestring ansible.module_utils.six.string_types or (ansible.module_utils.six.text_type, ansible.module_utils.six.binary_type) unicode ansible.module_utils.six.text_type or (ansible.module_utils._text.to_native, ansible.module_utils._text.to_text) [@]_ bytes ansible.module_utils.six.binary_type or (ansible.module_utils._text.to_bytes, ansible.module_utils._text.to_native) [@]_ iteritems ansible.module_utils.six.iteritems reduce ansible.module_utils.six.moves import reduce NUMBERTYPES ansible.module_utils.six.integer_types + (float,) [+]_ NoneType ansible.module_utils.pycompat27.NoneType ## Need to move Sequence ansible.module_utils.pycompat24.Sequence ## Need to move Mapping ansible.module_utils.pycompat24.Mapping ## Need to move SEQUENCETYPE ansible.module_utils.pycompat27.SEQUENCETYPE ## Need to move json ansible.module_utils.pycompat24.json ## Need to move literal_eval ansible.module_utils.pycompat24.literal_eval get_exception ansible.module_utils.pycompat24.get_exception

if not isinstance(variable, basestring):

# Get the string representation of the object variable = str(variable)

if not isinstance(variable, unicode):

# Make sure a string is unicode for the API we're calling variable = unicode(variable, 'utf-'8')

Those should be replaced with code like this:

if not isinstance(variable, (six.binary_type, six.text_type)):
    # Get the string representation of the object
    variable = to_native(variable, errors='surrogate_or_strict')

if not isinstance(variable, six.text_type):
    # Make sure a string is the platform's text type (unicode on py2, str on py3)
    variable = to_text(variable, 'utf-8')

For converting between text and bytes use ansible.module_utils._text.to_text and ansible.module_utils._text.to_bytes:

# Original:
text_string = unicode("Toshio wrote this", encoding='utf-8')
byte_string = bytes(u"Toshio wrote this", 'utf-8')

# New:
text_string = to_text("Toshio wrote this", errors='surrogate_or_strict')
byte_string = to_bytes(u"Toshio wrote this", errors='surrogate_or_strict')

For converting between an unknown object and its string representation (for instance, for printing an informational error message), use the nonstring argument to to_text and `to_bytes`:

# Original:
text_msg = u'Error: Can not process: %s' % unicode({'An object': 'value'}, encoding='utf-8')
byte_msg = 'Error: Can not process: %s' % bytes({'An object': 'value'}, 'utf-8')

# New:
text_msg = u'Error: Can not process: %s' % to_text({'An object': 'value'}, nonstring='simplerepr')
byte_msg = to_bytes('Error: Can not process: %s' % to_native({'An object': 'value'}, nonstring='simplerepr'))
# byte_msg is different because the original was not safe on python3

For testing whether a value is a text or byte string:

# Original:
if isinstance('string', unicode): pass
if isinstance('string', bytes): pass

# New:
if isinstance('string', text_type): pass
if isinstance('string', binary_type): pass





# Old code:
from ansible.module_utils.basic import NUMBERTYPES
if isinstance(obj, NUMBERTYPES):
    return str(obj)

from ansible.module_utils.six import integer_types
# New code:
if isinstance(obj, integer_types + (float,)):
    return str(obj)

Move the deprecated values that belong in pycompatXY into those files.

Ansible Core Team -- do we want to make an ansible.module_utils.json with this and other json-related functions? (Or put this into a pycompat* as it's only for python-2.4 which doesn't have the stdlib json library)?

Private Globals

Rename these to have a leading underscore to mark them as private

RST Notes

If we switch to inline documentation, attributes are documented like this:

#: All the values that parameter parsing converts to True
BOOLEANS_TRUE = ['yes', 'on', '1', 'true', 'True', 1, True]

#: All the values that parameter parsing converts to False
BOOLEANS_FALSE = ['no', 'off', '0', 'false', 'False', 0, False]

Pending

This list of functions needs to be further documented

Move to _text

Logging related

Formatting of strings

Move to _text heading. Should we make text.converters for what's there currently and text.formatters for these?

File handling

Hashing

Module Parameter handling

Module Return handling

On Module Start

On Module Exit

Process management

Json

Uncategorized

AnsibleModule

Version 2 of AnsibleModule should contain functionality that sets up a Module similar to how GUI frameworks have an App class that sets up an Application environment. Parameter parsing and registering how to exit from the Module seem like things which are AnsibleModule responsibilities.

  • Entrypoint for current AnsibleModules
  • Going to need a new AnsibleModule
  • Going to split most methods out of it
  • New focus should be
    • parameter handling.
    • return values?
    • log_sanitization?
  • Initialization is all about handling parameters

From AnsibleModule

Same categories as for the functions. Many of these will need to be ported from being methods to being functions.

Move to _text

Logging related

Formatting of strings

File handling

===> hashing

Module Parameter handling

Module Return handling

On Module Start

On Module Exit

Process management

Json

Uncategorized

  • common/
  • common/__init__.py
  • common/module.py
  • common/file.py
  • common/hashing.py
  • common/json.py
  • common/logging.py
  • common/process.py
  • common/sys_info.py
  • common/connection.py
  • params/__init__.py
  • params/common.py
  • params/size_validators.py
  • params/serialized_validators.py
  • text/
  • text/__init__.py
  • text/convert.py
  • text/format.py

common/module.py ==============

  • Contains an AnsibleModule class
  • Parses parameters
  • Sets up the Module Environment (exit functions, cleanup functions)
  • It's expected that all modules will import core/module.py (either directly or through a dependency).

common/file.py ============ .. py:data:: PERM_BITS .. py:data:: EXEC_PERM_BITS .. py:data:: DEFAULT_PERM

  • Are these private? (and should be marked as such)

common/hashing.py =============== .. py:data:: AVAILABLE_HASH_ALGORITHMS .. py:meth:: digest_from_file(self, filename, algorithm)

Note

Perhaps this file should all be deprecated

common/json.py ============ .. py:class:: _SetEncoder(json.JSONEncoder) .. py:meth:: jsonify(self, data) .. py:meth:: from_json(self, data)

Also include things in json_utils.py

common/logging.py =============== .. py:data:: PASSWD_ARG_RE .. py:function:: return_values(obj: Any) -> (iterator)NativeStrings .. py:function:: _remove_values_conditions(value: Any, no_log_strings: Set, deferred_removals: List) -> Any .. py:function:: remove_values(value: Any, no_log_strings: Set) -> Any .. py:function:: heuristic_log_sanitize(data: String, no_log_values=None: Set) -> NativeString

Note

These are all used by AnsibleModule when returning information so perhaps they should remain in core/module.py

common/process.py =============== .. py:meth:: get_bin_path(self, arg, required=False, opt_dirs=[]) .. py:meth:: _read_from_pipes(self, rpipes, rfds, file_descriptor) .. py:meth:: run_command(self, args, check_rc=False, close_fds=True, executable=None, data=None, binary_data=False, path_prefix=None, cwd=None, use_unsafe_shell=False, prompt_regex=None, environ_update=None, umask=None, encoding='utf-8', errors='surrogate_or_strict')

common/sys_info.py ================ .. py:function:: get_platform() -> NativeString .. py:function:: get_distribution() -> Union[NativeString, None] .. py:function:: get_distribution_version() -> Union[NativeString, None]

common/connection.py ================

Move the toplevel connection.py file to here.

params/common.py ============== Put parameter parsing and validation into its own module so that we can use it for action plugins in addition to modules. ansible.module_utils.core.module will have to import it, though.

params/size_validators.py

params/serialized_validators.py

text/convert.py

text/format.py

@alikins
Copy link

alikins commented Sep 18, 2017

.. warn:: Core team members, do we want to change this API? Instead of
returning an instantiated class, return the class and make the caller
instantiate it. Doing that will mean we don't have to pass in *args
and **kwargs.

I like returning the class better. I would say that load_platform_subclass() needs to be split into a method that finds matching classes, and possibly a method that instantiates the found classes.

The facts code ended up doing something similar to avoid
the metaclass bits it was using. The facts code is mostly in https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/facts/collector.py#L166 and https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/facts/collector.py#L46 .

That approach also doesn't require all of the platform impls to be subclasses of the same type. Doesn't require any type checking at all for that matter, though it does expect a certain interface ala duck typing.

The Collector.platform_match() allows FactCollector subclasses[1] define how they 'match' a platform. Though all current cases use the base version. That also lets Collector classes decide if they are a platform match based on local criteria (for ex, match on 'Linux' if python3, but on python2 match on 'Generic' without involving setup). The collector.platformat_match() gets to decide

[1] or any class that provides the same interface

@privateip
Copy link

I think it makes sense to include connection in the core package. The module_utils/connection.py is how modules that use persistent connection connects back to the local domain socket when the module runs.

@sivel
Copy link

sivel commented Jan 2, 2018

I've started some of this work which you can see at ansible/ansible@devel...sivel:categorization-of-basic

Some notes:

  1. The new AnsibleModule doesn't have the same signature, as it now expects to be passed a callable for the param validator which is it's own class, and instantiated separately.
  2. The param validator should be backwards compat, but due to it being it's own class and AnsibleModule needing access to some internals, I've exposed those as attributes. This allows for a pluggable validator to be used, but we will only ship AnsibleParamsValidator for now. Some sanity checks are needed.
  3. I have started copying things out of basic.py and largely where they were methods, they are encapsulated in a DummyClass right now, but that needs resolved.
  4. For the things I have copied out of basic.py, we should remove from basic.py and import from the new correct locations. Backwards compat in mind for what seems reasonable. I've tried to document some TODO lines in basic.py and elsewhere that need remediated as a result of the splits, but not everywhere.
  5. This is of course not done, or even close

@bcoca
Copy link

bcoca commented Oct 17, 2018

  • add daemonization (we have 2-3 implementaitons) to process management, as it can share many innards with run_command (we can split out into internal functions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment