Contributing to Placement

Contributing to Placement

As an official OpenStack project, Placement follows the overarching processes outlined in the Project Team Guide and Developer’s Guide. Contribution is welcomed from any interested parties and takes many different forms.

To make sure everything gets the attention it deserves and work is not duplicated there are some guidelines, stated here.

If in doubt, ask someone, either by sending a message to the openstack-discuss mailing list with a [placement] subject tag, or by visiting the #openstack-placement IRC channel on chat.freenode.net.

StoryBoard

Placement uses StoryBoard for tracking bugs, features, and “cleanups” (described below). Different types of stories are distinguished by tags which are applied manually by people creating or reviewing the stories. These tags are used to create work lists.

Worklists

List

Main Tag

Description

Untriaged Stories

No tags

New or unvisited stories which have not yet been categorized (and should be).

Bugs

bug

Incorrect behavior in existing code, features or documents. If the issue is with documentation, add the tag docs.

Features

rfe

Planned or requested features or enhancements to the system.

Cleanups

cleanup

Improvements to the code or surrounding repository to help with maintenance or other development lifecycle issues that are not otherwise a bug or feature.

When a story has been completed it can be removed from the list by being tagged with fixed.

Note

These worklists and their chosen tags are subject to change as the project gains more experience with StoryBoard. The tags chosen thus far are, in part, the result of them already existing elsewhere in the system.

Submitting and Managing Bugs

Bugs found in placement should be reported in StoryBoard by creating a new story in the openstack/placement project.

New Bugs

If you are submitting a new bug, explain the problem, the steps taken that led to the bad results, and the expected results. Please also add as much of the following information as possible:

  • Relevant lines from the placement-api log.

  • The OpenStack release (e.g., Stein).

  • The method used to install or deploy placement.

  • The operating system(s) on which the placement is running.

  • The version(s) of Python being used.

Tag the story with bug.

Triaging Bugs

Triaging newly submitted bugs to confirm they really are bugs, gather missing information, and to suggest possible solutions is one of the most important contributions that can be made to any open source project.

If a new bug doesn’t have the bug tag, add it. If it isn’t a functional problem with existing code, consider adding the rfe tag for a feature request, or cleanup for a suggested refactoring or a reminder for future work. An error in documentation is a bug.

Leave comments on the story if you have questions or ideas. If you are relatively certain about a solution, add the steps of that solution as tasks on the story.

If submitting a change related to a story, the gerrit system will automatically link to StoryBoard if you include Story: and, optionally, Task: identifiers in your commit message, like this:

Story: 2005190
Task: 29938

Using solely Story: will leave a comment referring back to the commit in gerrit. Adding Task: will update the identified task to indicate it is in a Review state. When the change merges, the state will be updated to Merged.

Reviewing Code

Like other OpenStack projects, Placement uses gerrit to facilitate peer code review. It is hoped and expected that anyone who would like to commit code to the Placement project will also spend time reviewing code for the sake of the common good. The more people reviewing, the more code that will eventually merge.

See How to Review Changes the OpenStack Way for an overview of the review and voting process.

There is a small group of people within the Placement team called core reviewers. These are people who are empowered to signal (via the +2 vote) that code is of a suitable standard to be merged and is aligned with the current goals of the project. Core reviewers are regularly selected from all active reviewers based on the quantity and quality of their reviews and demonstrated understanding of the Placement code and goals of the project.

The point of review is to evolve potentially useful code to merged working code that is aligned with the standards of style, testing, and correctness that we share as group. It is not for creating perfect code. Review should always be constructive, encouraging, and friendly. People who contribute code are doing the project a favor, make it feel that way.

Some guidelines that reviewers and patch submitters should be aware of:

  • It is very important that a new patch set gets some form of review as soon as possible, even if only to say “we’ve seen this”. Latency in the review process has been identified as hugely discouraging for new and experienced contributors alike.

  • Follow up changes, to fix minor problems identified during review, are encouraged. We want to keep things moving.

  • As a reviewer, remember that not all patch submitters will know these guidelines. If it seems they don’t, point them here and be patient in the meantime.

  • Gerrit can be good for code review, but is often not a great environment for having a discussion that is struggling to resolve to a decision. Move discussion to the mailing list sooner rather than later. Add a link to the thread in the list archive to the review.

  • If the CI system is throwing up random failures in test runs, you should endeavor whenever possible to investigate, not simply recheck. A flakey gate is an indication that OpenStack is not robust and at the root of all this, making OpenStack work well is what we are doing.

Special Considerations For Core Reviewers

Core reviewers have special powers. With great power comes great responsibility and thus being held to a standard. As a core reviewer, your job is to enable other people to contribute good code. Under ideal conditions it is more important to be reviewing other people’s code and bugs and fixing bugs than it is to be writing your own features. Frequently conditions will not be ideal, but strive to enable others.

When there are open questions that need to be resolved, try to prefer the openstack-discuss list over IRC so that anyone can be involved according to their own schedules and input from unexpected sources can be available.

Writing Code

This document cannot enumerate all the many ways to write good Python code. Instead it lists some guidelines that, if followed, will help make sure your code is reviewed promptly and merges quickly. As with everything else in this document, these guidelines will evolve over time and may be violated for special circumstances. If you have questions, ask.

See Placement API Developer Notes for an overview of Placement and how the various pieces fit together.

  • Divide your change into a series of commits each of which encapsulates a single unit of functionality but still results in a working service. Smaller changes are easier to review.

  • If your change is to the HTTP API, familiarize yourself with Microversions.

  • If there is a series of changes leading to an HTTP API change, exposing that API change should be the last patch in the series. That patch must update the API reference and include a release note.

  • Changes must include tests. There is a separate document on Testing Placement.

  • Run tox before submitting your code to gerrit. This will run unit and functional tests in both Python 2 and Python 3, and pep8 style checks. Placement tests, including functional, are fast, so this should not be too much of a hardship. By running the tests locally you avoid wasting scarce resources in the CI system.

  • Keep the tests fast. Avoid sleeps, network connections, and external processes in the tests.

  • Keep Placement fast. There is a placement-perfload job that runs with every patch. Within that is a log file, /logs/placement-perf.txt[.gz] that gives rough timing information for a common operation. We want those numbers to stay small.

  • We follow the code formatting guidelines of PEP 8. Check your code with tox -epep8 (for all files) or tox -efast8 (for just the files you changed). You will not always agree with the advice provided. Follow it.

  • Where possible avoid using the visual indent style. Using it can make future changes unnecessarily difficult. This guideline is not enforced by pep8 and has been used throughout the code in the past. There’s no need to fix old use. Instead of this

    return_value = self.some_method(arg1, arg2,
                                    arg3, arg4)
    

    prefer this

    return_value = self.some_method(
        arg1, arg2, arg3, arg4)
    
  • Changes associated with stories and tasks in StoryBoard should include Story and Task identifiers in the commit message, as described in Triaging Bugs above.

New Features

New functionality in Placement is developed as needed to meet new use cases or improve the handling of existing use cases. As a service used by other services in OpenStack, uses cases often originate in those other services. Considerable collaboration with other projects is often required to determine if any changes are needed in the Placement API or elsewhere in the project. That interaction should happen in the usual ways: At Project Team Gatherings, on the openstack-discuss list, and in IRC.

Once there is a clear need for a change, a story should be created in StoryBoard with a tag of rfe. Placement team members will evaluate the story to determine if a spec is required. If it is, a task to create the spec will be added to the story. At this time there are no hard and fast rules on what will require a spec. If the implementation is well understood it may be the case that a detailed story and a series of tasks associated with that story will be sufficient. If further discussion is required to understand the problem or to evolve or verify the design of the solution, a spec is a good idea.

If a spec is required there are some guidelines for creating one:

  • A file should be created in the placement code in doc/source/specs/<cycle-name>/approved with a filename beginning with the identifier of the story. For example:

    docs/source/specs/train/approved/200056-infinite-resource-classes.rst
    

    More details on how to write a spec are included in a template file found in the approved directory of each release cycle.

  • Under normal circumstances specs should be proposed near the beginning of a release cycle so there is sufficient time to review the spec and its implementation as well as to make any necessary decisions about limiting the number of specs being worked in the same cycle. Unless otherwise announced at the beginning of a cycle, specs should merge before milestone-2 to be considered relevant for that cycle. Exceptions will be reviewed on a case by case basis. See the stein schedule for an example schedule.

  • Work items that are described in a spec should be reflected as tasks created on the originating story. Update the story with additional tasks as they are discovered. Most new tasks will not require updating the spec.

  • If, when developing a feature, the implementation significantly diverges from the spec, the spec should be updated to reflect the new reality. This should not be considered exceptional: It is normal for there to be learning during the development process which impacts the solution.

  • Though specs are presented with the Placement documentation and can usefully augment end-user documentation, they are not a substitute. Development of a new feature is not complete without documentation.

When a spec was approved in a previous release cycle, but was not finished, it should be re-proposed (via gerrit) to the current cycle. Include Previously-Approved: <cycle> in the commit message to highlight that fact. If there have been no changes, core reviewers should feel free to fast-approve (only one +2 required) the change.

Creative Commons Attribution 3.0 License

Except where otherwise noted, this document is licensed under Creative Commons Attribution 3.0 License. See all OpenStack Legal Documents.