Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAPEX-27: Implement CI/CD System #2

Merged
merged 18 commits into from
Sep 25, 2023
Merged

Conversation

mikecarenzo
Copy link
Contributor

@mikecarenzo mikecarenzo commented Sep 18, 2023

Implements #27

What Changed

This pull requests introduces a new CI/CD system.

New Project Structure

The src directory now maintains two completely independent packages.

  • cli: Contains the Mappings CLI Package.
  • explorer: Contains the Mappings Explorer Package.

packages

Each package defines their own:

File Purpose
Python Package mappings_* Contains the package's source code.
Tests tests Contains the package's pytests.
README File README.md Documents the package.
Build Information pyproject.toml Documents the build system's requirements and information.
Task Runner Configuration tasks.py See: New Task Runner

It's important that we strictly separate each package. Separating the packages allows us to track the changes made to each package individually. This allows us to maintain separate change logs, separate version numbers, and (most importantly) separate releases.

Users that pip install the Mappings CLI only need the CLI, not the Explorer. Users that read the CLI's release notes only care about changes made to the CLI. Users that upgrade the CLI from one version to the next expect the version number to change only when changes to the CLI have been made. The same holds true for users of the Mappings Explorer.

Does this mean there are two separate Poetry environments?

Yes, for now. However, this won't be an obstacle. Refer to Limitations for more information.

New Task Runner

make has been replaced with invoke.

While make is a powerful task runner, it's not pre-installed on Windows. We could instruct Windows developers to install it, but doing so would add a platform-specific step to the development process without offering any real advantages. make's syntax is generally foreign to most Python developers. As a result, Makefiles could prove difficult to maintain long-term.

invoke is a popular Python task runner designed specifically to emulate tools like make, but instead of relying on makefiles, it defines a tasks.py file.

To test a package, simply run:

$ invoke test 

To lint a package:

$ invoke lint

To lint AND test a package:

$ invoke lint test 

To list the available tasks:

$ invoke --list              
Available tasks:

  build   Build the Python package.
  lint    Run black, ruff, and mypy
  test    Run Python tests.

To get help on a specific task:

$ invoke --help test
Usage: inv[oke] [--core-opts] test [--options] [other tasks here ...]

Docstring:
  Run Python tests.

Options:
  -x, --xml   Include XML coverage report

It's recommended that you install invoke globally (pip install invoke). However, if you choose not to, you can also run invoke inside a poetry shell once you poetry install either project. You can also poetry run invoke.

For more information about tasks.py files, refer to What is Invoke?

Added release-please GitHub Action

Google's release-please automatically tracks commits made to the repository's main branch (in the Conventional Commit format). It uses the commit messages to compute each package's version number and generate each package's change log. release-please updates each version number and change log in a parallel "Release Pull Request". When we're ready to tag a release, we simply merge the Release PR.

For more information about Release PRs, refer to the documentation.

This PR also introduces the add_tags GitHub Action. It's a custom action that offers us more control over how and when GitHub Releases are published.

For more information about add_tags, refer to the README.

Improved Workflow Performance

Because there are two independent packages, there are two independent test workflows. Each workflow is designed to lint, run test cases, and assess security vulnerabilities. Separating the tests ensures that both tests run. If both the CLI and Explorer were tested in sequence (under the same workflow) a failure in the CLI tests would prevent the Explorer from being tested.

To improve test performance, both workflows now leverage GitHub's Cache action. The first time a test workflow runs, it installs Poetry and the package's dependencies. After the workflow completes, it caches the Poetry installation and the package's virtual environment. Then, on each consecutive run, the workflow restores the Poetry installation and virtual environment from the cache (instead of redownloading and reconfiguring the environment). This cuts down the execution time for both workflows.

Both workflows are configured to destroy their cached virtual environment if the package's poetry.lock file changes. In this case, it will reinstall and recache the virtual environment.

Updated Editor Configurations

Visual Studio Code has broken the Python extension into several separate extensions. As a result:

  • "python.formatting.autopep8Path"
  • "python.formatting.provider"

have been deprecated.

The settings.json file has been updated to reflect these new extensions and their settings.

An extensions.json file has also been added. This file declares all the project's recommended VSCode Extensions so that new developers can immediately configure their development environment the first time the project launches.

show_recommendations_1

If the user clicks Show Recommendations, they'll be shown the recommended plugins:

show_recommendations_2

Limitations

Both pyproject.toml files share a similar set of dependencies, configurations, and build options. Unfortunately, Poetry has limited support for monorepos. Although, that could be changing [1] [2].

In the future, I'd like to develop a small script that unifies the virtual environments into a singular environment during development. Until then, they'll need to be separate. Both projects serve largely different purposes so this really shouldn't be much of an issue. There's no real case where a developer would need to frequently switch back and forth between the two environments.

@mikecarenzo mikecarenzo requested a review from mehaase September 18, 2023 14:02
Copy link
Contributor

@mehaase mehaase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a LOT of changes here, which is great, but I don't want to roll out so many big changes all at the
same time. Let's prioritize a few ideas to try on this project, and let's defer the rest to future projects.

  • Release please sounds like a great addition to our workflow but I left some notes on the documentation.
  • While I like the idea that we could do a monorepo in the future, I don't want to separate the cli and
    mappings_explorer packages. Let's keep those in one python package.
  • I'm not convinced about switching from make to invoke. Let's keep make for now.
  • VS Code settings / extensions is great idea

I left questions on a few files.

.github/actions/add_tags/dist/index.js Outdated Show resolved Hide resolved
.github/workflows/python_test_cli.yml Outdated Show resolved Hide resolved
.github/actions/add_tags/index.js Outdated Show resolved Hide resolved
.release-please-manifest.json Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
src/cli/pyproject.toml Outdated Show resolved Hide resolved
@mikecarenzo mikecarenzo merged commit 971c9b2 into main Sep 25, 2023
1 check passed
@mikecarenzo mikecarenzo deleted the MAPEX-27_implement_cicd branch September 25, 2023 16:02
mikecarenzo added a commit that referenced this pull request Apr 15, 2024
…add_m365_mappings

SSM-15: Add M365 Mappings to Explorer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants