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

Add GHA workflow to lint with pantsbuild #5758

Merged
merged 3 commits into from
Oct 8, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
---
# This Lint workflow uses pants
name: Lint

on:
push:
branches:
# only on merges to master branch
- master
# and version branches, which only include minor versions (eg: v3.4)
- v[0-9]+.[0-9]+
tags:
# also version tags, which include bugfix releases (eg: v3.4.0)
- v[0-9]+.[0-9]+.[0-9]+
pull_request:
type: [opened, reopened, edited]
branches:
# Only for PRs targeting those branches
- master
- v[0-9]+.[0-9]+
schedule:
# run every night at midnight
- cron: '0 0 * * *'

Choose a reason for hiding this comment

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

Why this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied that from our other workflows which we run every day. We did that in part to catch issues with transitive deps, which will not be a significant issue once we've got the lockfiles in. So, I suppose it is unlikely for linters to fail from one day to another, so maybe we don't have to be quite so aggressive here. 🤔

What do others think? Drop this? Comment it out for now?

Choose a reason for hiding this comment

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

which will not be a significant issue once we've got the lockfiles in.

Note that you already are using "tool lockfiles". Pants ships them by default. So your installation of tools like Flake8 and Pylint will be deterministic. It is only installs of your own user code that need to manually set up lockfiles, since we can't ship them without knowing before what your code is.

So running this is cron is likely a waste of compute resources (and trees)

Copy link
Member Author

Choose a reason for hiding this comment

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

I commented it out.


jobs:
# Lint checks which don't depend on any service containes, etc. to be running.
lint-checks:
name: 'Lint Checks (pants runs: shellcheck)'
runs-on: ubuntu-latest

env:
COLUMNS: '120'

steps:
- name: Checkout repository
uses: actions/checkout@v2
with:
# a test uses a submodule, and pants needs access to it to calculate deps.
submodules: 'true'

#- name: Cache APT Dependencies
# id: cache-apt-deps
# uses: actions/cache@v2
# with:
# path: |
# ~/apt_cache
# key: ${{ runner.os }}-apt-v7-${{ hashFiles('scripts/github/apt-packages.txt') }}
# restore-keys: |
# ${{ runner.os }}-apt-v7-
- name: Install APT Depedencies
env:
CACHE_HIT: 'false' # cache doesn't work
#CACHE_HIT: ${{steps.cache-apt-deps.outputs.cache-hit}}
run: |
# install dev dependencies for Python YAML and LDAP packages
# https://github.com/StackStorm/st2-auth-ldap
./scripts/github/install-apt-packages-use-cache.sh

- name: Initialize Pants and its GHA caches
uses: pantsbuild/actions/init-pants@c0ce05ee4ba288bb2a729a2b77294e9cb6ab66f7
Copy link
Member

Choose a reason for hiding this comment

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

can't we point to master or latest release or something here

Copy link
Member Author

Choose a reason for hiding this comment

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

This repo does not yet have tags we can point to. If there are changes, we might need to adjust how we use the action, app pinning to a particular version seemed the best way to go.

When we choose another approach (eg when there are tags available), then we'll need to change the pants BUILD validation workflow add well:

uses: pantsbuild/actions/init-pants@c0ce05ee4ba288bb2a729a2b77294e9cb6ab66f7

# This action adds an env var to make pants use both pants.ci.toml & pants.toml.
# This action also creates 3 GHA caches (1 is optional).
# - `pants-setup` has the bootsrapped pants install
# - `pants-named-caches` has pip/wheel and PEX caches
# - `pants-lmdb-store` has the fine-grained process cache.
# If we ever use a remote cache, then we can drop this.
# Otherwise, we may need an additional workflow or job to delete old caches
# if they are not expiring fast enough, and we hit the GHA 10GB per repo max.
with:
base-branch: master
# To ignore a bad cache, bump the cache* integer.
gha-cache-key: cache0
# This hash should include all of our lockfiles so that the pip/pex caches
# get invalidated on any transitive dependency update.
named-caches-hash: ${{ hashFiles('requirements.txt') }}
Copy link
Member

Choose a reason for hiding this comment

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

is the root level requirements.txt enough in this scenario?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not entirely. I plan to change this to use the lockfiles instead once the lockfiles are available.

# enable the optional lmdb_store cache since we're not using remote caching.
cache-lmdb-store: 'true'

- name: Lint
run: |
./pants lint ::

- name: Upload pants log
uses: actions/upload-artifact@v2
with:
name: pants-log-py${{ matrix.python-version }}
path: .pants.d/pants.log
if: always() # We want the log even on failures.
2 changes: 1 addition & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Added

* Begin introducing `pants <https://www.pantsbuild.org/docs>`_ to improve DX (Developer Experience)
working on StackStorm, improve our security posture, and improve CI reliability thanks in part
to pants' use of PEX lockfiles. This is not a user-facing addition. #5713 #5724 #5726 #5725 #5732 #5733 #5737 #5738
to pants' use of PEX lockfiles. This is not a user-facing addition. #5713 #5724 #5726 #5725 #5732 #5733 #5737 #5738 #5758
Contributed by @cognifloyd

Changed
Expand Down