Atmosphere Code Guidelines

These guidelines will improve the consistency and comprehensibility of our code. Contributions to "the Atmosphere projects" (atmosphere, troposphere, atmosphere-ansible, clank, etc) will be checked for adherence.

• Delete your dead code. If you know that code is not run, don't comment it out, instead remove it completely. Old code remains in version control and we can dig into the past if needed.

Ansible Style Guidelines

These apply to repos such as clank and atmosphere-ansible (collectively, "the Ansible projects"). Some are borrowed from Open edX Operations and OpenStack-Ansible.

YAML Files and Formatting

YAML files should:

• Have a name ending in .yml
• Begin with three dashes followed by a blank line
• Use 2-space indents
• Use YAML dictionary format rather than in-line key=values. It's easier to read and reduces changeset collisions in version control. So, do this:

- name: foo
  file:
    src: /tmp/bar
    dest: /tmp/baz
    state: present

Not this:

- name: foo src=/tmp/bar dest=/tmp/baz state=present

• Use YAML folded style for long lines:

- shell: >
        python a very long command --with=very --long-options=foo
        --and-even=more_options --like-these

Variables

• Use spaces between double-braces and variable names: {{ var }} instead of {{var}}
• For boolean variables, use un-quoted true and false
• Bare variables in YAML must be quoted, e.g.

- foo: "{{ bar }}"

• For variables that have some default and are optionally overridden by a consumer/deployer: define the variable's default value in the role defaults/ instead of using the | default() jinja2 filter. This keeps configuration separate from code.

Distro-Specific Variables

Many roles must take differently-configured actions to effect the same result, depending on the target's operating system. Accomplish this configuration by defining your variables in files like so:

vars/
├── CentOS-5.yml
├── CentOS.yml
├── Ubuntu-12.yml
├── Ubuntu-16.yml
└── Ubuntu.yml

Use the following task to include the appropriate variables file:

- name: gather OS-specific variables
  include_vars: "{{ item }}"
  with_first_found:
    - "{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml"
    - "{{ ansible_distribution }}.yml"

Matching is most-specific to least-specific, so with the above example, a role targeting an Ubuntu 16.04 system would include the variables defined in Ubuntu-16.yml, while a role targeting Ubuntu 14.04 would include the variables defined in Ubuntu.yml. Also, only one file will match and there is no concept of inheritance (so usually, the same variables should be defined in each file).

Role-specific variables that do not vary per-distro can still be defined in vars/main.yml or defaults/main.yml.

Tasks

Name your tasks to reflect the completed action in the past tense: denyhosts stopped and disabled rather than stop and disable denyhosts. This reinforces the goal of defining a series of idempotent steps to either reach or confirm a desired end state.

Conditional Logic

• To conditionally include or exclude a role, use the following syntax in a playbook:

roles: - { role: ldap, when: CONFIGURE_LDAP == true }

Avoid applying the same tag to every task in a role and expecting a user to skip a role with --skip-tags.

Modules

• Use command rather than shell unless shell is explicitly required (source 0 source 1).

Files and Installation Packages

Avoid storing large binary files or installation packages in Ansible projects / version control. Instead, host it externally (e.g. CyVerse's S3 account) and download from there, or download from the publisher.

Possible Ansible Guidelines - Things to Discuss

• Quote all strings and prefer single quotes over double quotes? This makes clear what is a string and what refers to a variable
• Define a general order for a task declaration?
• Variables that are internal to a role should be in snake_case? Variables that are environment-specific and should be overridden in UPPERCASE?
• When to use a role's vars/ vs. defaults/ directory? vars/ for things internal to the role (e.g. distro-specific), defaults/ for things expected to be overridden by a consumer?
• Prefix all variables defined in a role with the name of the role to avoid collisions?
• Prefix task names with the role name, to ease human parsing of log output?
• Separate words in role names with dashes (my-role)?

Ansible Galaxy Roles

The Ansible projects will use similar code to perform very similar tasks, so there is an effort to structure our code into modular, re-usable roles. These roles are generally hosted in the CyVerse-Ansible organization and consumed via Ansible Galaxy. This will centralize the development/maintenance effort and provide easy consumption by any interested groups. New Ansible contributors are encouraged to read Ansible Galaxy Intro.

Identifying and Working With Galaxy Roles

The Ansible projects include a mix of roles installed from Galaxy and roles defined locally. The following clues should indicate that a given role was installed from Galaxy.

• The role is defined in the project's requirements.yml
• The role contains meta/galaxy_install_info and meta/main.yml

There is a clear concept of "upstream" (roles) and "downstream" (projects that consume them), so generally we should avoid making persistent changes to a role which has been installed from Galaxy inside a repository that consumes it. Such changes will 1. not propagate back upstream to the role repository, and 2. be overwritten the next time the role is updated from Galaxy. Instead, make your changes in the upstream Ansible role (see below), re-import it, and install the new version in the consuming repository.

Installing (Consuming) Galaxy Roles in Another Project

The Ansible projects use a requirements.yml file to define the Galaxy roles we are consuming. Install new roles and update existing roles in a repo by running ansible-galaxy install -f -r requirements.yml -p roles/ from the ansible folder. Then, the desired roles will be copied into your project, and you can call them from Playbooks like any other role.

Importing (Contributing) a Role to Galaxy

Follow the instructions in this template to publish an Ansible role to Galaxy and set up automated testing with Travis CI; then you can install the role into other projects.

When you're ready to add your project to the CyVerse-Ansible organiztion contact Chris Martin for an invite.

Note for maintainers: It is suggested that roles in Cyverse-Ansible be versioned with tags. Be cognizant to push tags when the project changes, and please follow the SemVer protocol.

Security Guidelines

Do not store anything secret or sensitive (e.g. passphrases, SSH private keys, license keys) in public version control. Environment-specific secrets can be encrypted with Ansible Vault and stored in private version control.

When using ansible-vault, consider the following suggestions: * Use one password per repository * Vaulted file names should be pre-fixed or suffixed with "vault" - vault-sample-file-name.yml - sample-file-name-vault.yml

Any REST API calls that handle privileged/sensitive information (or access to private resources) should use HTTPS endpoints. Also, any downloaded code that will be executed in a trusted context (e.g. scripts and installation packages) should be obtained in a way that verifies the other party's authenticity and prevents man-in-the-middle attacks from changing the contents. In practical terms, use HTTPS, or APT packages signed with a GPG key. Avoid disabling SSL certificate verification for any of the above.

Avoid disabling SSH host key checking. When possible, learn a server's SSH host key out-of-band and verify it on first connection.

Contribution Guide Contribution Guide

If you ask someone not to do something, offer a reasonable alternative approach. Otherwise, they may ignore your advice for the sake of pragmatism.

Further Reading

• Code Smells

Python Style Guidelines

This Python style guide is largely copied from Center for Open Science

Follow PEP8, when sensible.

Naming

• Variables, functions, methods, packages, modules
  • lower_case_with_underscores
• Classes and Exceptions
  • CapWords
• Protected methods and internal functions
  • _single_leading_underscore(self, ...)
• Private methods
  • __double_leading_underscore(self, ...)
• Constants
  • ALL_CAPS_WITH_UNDERSCORES

General Naming Guidelines

Use singlequotes for strings, unless doing so requires lots of escaping.

Avoid one-letter variables (esp. l, O, I).

Exception: In very short blocks, when the meaning is clearly visible from the immediate context

for e in elements:
    e.mutate()

Avoid redundant labeling.

# Yes
import audio

core = audio.Core()
controller = audio.Controller()

# No
import audio

core = audio.AudioCore()
controller = audio.AudioController()

Prefer "reverse notation".

# Yes
elements = ...
elements_active = ...
elements_defunct = ...

# No
elements = ...
active_elements = ...
defunct_elements ...

Avoid getter and setter methods.

# Yes
person.age = 42

# No
person.set_age(42)

Indentation

Use 4 spaces--never tabs. You may need to change the settings in your text editor of choice.

Imports

Import entire modules instead of individual symbols within a module. For example, for a top-level module canteen that has a file canteen/sessions.py,

# Yes

import canteen
import canteen.sessions
from canteen import sessions

# No
from canteen import get_user  # Symbol from canteen/__init__.py
from canteen.sessions import get_session  # Symbol from canteen/sessions.py

Exception: For third-party code where documentation explicitly says to import individual symbols.

Rationale: Avoids circular imports. See here.

Put all imports at the top of the page with three sections, each separated by a blank line, in this order:

1. System imports
2. Third-party imports
3. Local source tree imports

Rationale: Makes it clear where each module is coming from.

If you have intentionally have an unused import that exists only to make imports less verbose, be explicit about it. This will make sure that someone doesn't accidentally remove the import (not to mention that it keeps linters happy)

from my.very.distant.module import Frob

Frob = Frob

String formatting

Prefer str.format to "%-style" formatting.

# Yes
'Hello {}'.format('World')
 # OR
'Hello {name}'.format(name='World')

# No

'Hello %s' % ('World', )

Print statements

Use the print() function rather than the print keyword (even if you're using Python 2).

# Yes
print('Hello {}'.format(name))

# No
print 'Hello %s ' % name

Documentation

Follow PEP257's docstring guidelines. reStructured Text and Sphinx can help to enforce these standards.

All functions should have a docstring - for very simple functions, one line may be enough:

"""Return the pathname of ``foo``."""

Multiline docstrings should include:

• Summary line
• Use case, if appropriate
• Args
• Return type and semantics, unless None is returned

"""Train a model to classify Foos and Bars.

Usage::

    >>> import klassify
    >>> data = [("green", "foo"), ("orange", "bar")]
    >>> classifier = klassify.train(data)

:param train_data: A list of tuples of the form ``(color, label)``.
:return: A trained :class:`Classifier <Classifier>`
"""

Notes

• Use action words ("Return") rather than descriptions ("Returns").
• Document __init__ methods in the docstring for the class.

class Person(object):
    """A simple representation of a human being.

    :param name: A string, the person's name.
    :param age: An int, the person's age.
    """
    def __init__(self, name, age):
        self.name = name
        self.age = age

On Comments

Use them sparingly. Prefer code readability to writing a lot of comments. Often, small methods and functions are more effective than comments.

# Yes
def is_stop_sign(sign):
    return sign.color == 'red' and sign.sides == 8

if is_stop_sign(sign):
    stop()

# No
# If the sign is a stop sign
if sign.color == 'red' and sign.sides == 8:
    stop()

When you do write comments, use them to explain why a piece code was used, not what it does.

Method Overrides

One useful place for comments are method overrides.

class UserDetail(generics.RetrieveUpdateAPIView, UserMixin):

    # overrides RetrieveUpdateAPIView
    def get_serializer_context(self):
        return {'request': self.request}

Calling Superclasses' Methods

Use super when there is only one superclass.

class Employee(Person):

    def __init__(self, name):
        super(Employee, self).__init__(name)
        # or super().__init__(name) in Python 3
        # ...

Call the method directly when there are multiple superclasses.

class DevOps(Developer, Operations):

    def __init__(self):
        Developer.__init__(self)
        # ...

Line lengths

Don't stress over it. 80-100 characters is fine.

Use parentheses for line continuations.

wiki = (
    "The Colt Python is a .357 Magnum caliber revolver formerly manufactured "
    "by Colt's Manufacturing Company of Hartford, Connecticut. It is sometimes "
    'referred to as a "Combat Magnum". It was first introduced in 1955, the '
    "same year as Smith & Wesson's M29 .44 Magnum."
)

Recommended Syntax Checkers

We recommend using a syntax checker to help you find errors quickly and easily format your code to abide by the guidelines above. Flake8 is our recommended checker for Python. It will check for both syntax and style errors and is easily configurable. It can be installed with pip: :

$ pip install flake8

Once installed, you can run a check with: :

$ flake8

There are a number of plugins for integrating Flake8 with your preferred text editor.

Vim

• syntastic (multi-language)

Sublime Text

• Sublime Linter with SublimeLinter-flake8 (must install both)

It is recommended that you use the git pre-commit hook to ensure your code is compliant with our style guide.

ln -s $(pwd)/contrib/pre-commit.hook $(pwd)/.git/hooks/pre-commit

To automate running tests before a push use the git pre-push hook to ensure your code passes all the tests.

ln -s $(pwd)/contrib/pre-push.hook $(pwd).git/hooks/pre-push

When master is pulled, it's helpful to know if a pip install or a manage.py migrate is necessary. To get other helpful warnings:

ln -s $(pwd)/contrib/post-merge.hook $(pwd)/.git/hooks/post-merge

Credits

• Center for Open Science
• PEP8 (Style Guide for Python)
• Pythonic Sensibilities
• Python Best Practice Patterns