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 CI actions #11

Merged
merged 15 commits into from
Dec 6, 2023
96 changes: 96 additions & 0 deletions .github/workflows/psm-interop.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
name: PSM Interop

env:
# Force the stdout and stderr streams to be unbuffered
PYTHONUNBUFFERED: 1

on:
pull_request:
push:
branches:
- main

permissions:
contents: read

jobs:
unittest:
# By default, only version is printed out in parens, e.g. "unittest (3.10)"
# This changes it to "unittest (python3.10)"
name: "unittest (python${{ matrix.python_version }})"
runs-on: ubuntu-latest
strategy:
matrix:
python_version: ["3.9", "3.10", "3.11"]
fail-fast: false
permissions:
pull-requests: read # Used by paths-filter to read the diff.
contents: read
defaults:
run:
working-directory: './'

steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v4
with:
python-version: "${{ matrix.python_version }}"
cache: 'pip'
cache-dependency-path: './requirements.lock'

- name: "Install requirements"
run: |
pip list
pip install --upgrade pip setuptools
pip list
pip install -r requirements.lock
pip list

- name: "Generate protos"
run: >
python -m grpc_tools.protoc --proto_path=.
XuanWang-Amos marked this conversation as resolved.
Show resolved Hide resolved
--python_out=. --grpc_python_out=.
protos/empty.proto
Copy link
Member

Choose a reason for hiding this comment

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

We probably should keep the packages path after protos/: protos/grpc/testing.
@gnossen - curious what you'd recommend here. Does the path really matter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What's the benefits of keeping it? I thought those are just paths to the .proto files.

Copy link
Member

@sergiitk sergiitk Nov 21, 2023

Choose a reason for hiding this comment

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

They do declare package grpc.testing;, and IRRC file path should correspond to the packages.
In practice, this means that the imports will look different:

from protos.grpc.testing import messages_pb2

instead of

from protos import messages_pb2

Then if we need to embed other protos (f.e. xds), they'd be in their own package too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Checked with Richard offline, it's better to move them to correct location.

protos/messages.proto
protos/test.proto

- name: "Run unit tests"
run: python -m tests.unit

black-check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: psf/black@stable
with:
options: "--check --config black.toml"
XuanWang-Amos marked this conversation as resolved.
Show resolved Hide resolved
XuanWang-Amos marked this conversation as resolved.
Show resolved Hide resolved
version: "23.3.0"

isort-check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: isort/isort-action@v1
with:
configuration: "--diff --check-only --dont-follow-links --settings-path=black.toml"
XuanWang-Amos marked this conversation as resolved.
Show resolved Hide resolved
isort-version: "5.9.2"

pylint-check:
runs-on: ubuntu-latest
steps:
XuanWang-Amos marked this conversation as resolved.
Show resolved Hide resolved
- uses: actions/checkout@v4
- uses: actions/setup-python@v4
with:
python-version: "3.9"
cache: 'pip'

- name: Install dependencies
run: |
pip list
python -m pip install --upgrade pip
pip install pylint==2.2.2 astroid==2.3.3 toml==0.10.2 "isort>=4.3.0,<5.0.0"
sergiitk marked this conversation as resolved.
Show resolved Hide resolved
pip list
- name: Analysing the code with pylint
sergiitk marked this conversation as resolved.
Show resolved Hide resolved
run: |
python -m pylint 'bin' 'framework' --rcfile=.pylintrc -rn
python -m pylint 'tests' --rcfile=tests/pylintrc-tests -rn
93 changes: 93 additions & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
[VARIABLES]

# TODO(https://github.com/PyCQA/pylint/issues/1345): How does the inspection
# not include "unused_" and "ignored_" by default?
dummy-variables-rgx=^ignored_|^unused_|_

[DESIGN]

# NOTE(nathaniel): Not particularly attached to this value; it just seems to
# be what works for us at the moment (excepting the dead-code-walking Beta
# API).
max-args=14
max-parents=8

[MISCELLANEOUS]

# NOTE(nathaniel): We are big fans of "TODO(<issue link>): " and
# "NOTE(<username or issue link>): ". We do not allow "TODO:",
# "TODO(<username>):", "FIXME:", or anything else.
notes=FIXME,XXX

[MESSAGES CONTROL]

disable=
# TODO(https://github.com/PyCQA/pylint/issues/59#issuecomment-283774279):
# Enable cyclic-import after a 1.7-or-later pylint release that
# recognizes our disable=cyclic-import suppressions.
cyclic-import,
# TODO(https://github.com/grpc/grpc/issues/8622): Enable this after the
# Beta API is removed.
duplicate-code,
# TODO(https://github.com/grpc/grpc/issues/261): Doesn't seem to
# understand enum and concurrent.futures; look into this later with the
# latest pylint version.
import-error,
# TODO(https://github.com/grpc/grpc/issues/261): Enable this one.
# Should take a little configuration but not much.
invalid-name,
# TODO(https://github.com/grpc/grpc/issues/261): This doesn't seem to
# work for now? Try with a later pylint?
locally-disabled,
# NOTE(nathaniel): What even is this? *Enabling* an inspection results
# in a warning? How does that encourage more analysis and coverage?
locally-enabled,
# NOTE(nathaniel): We don't write doc strings for most private code
# elements.
missing-docstring,
# NOTE(nathaniel): In numeric comparisons it is better to have the
# lesser (or lesser-or-equal-to) quantity on the left when the
# expression is true than it is to worry about which is an identifier
# and which a literal value.
misplaced-comparison-constant,
# NOTE(nathaniel): Our completely abstract interface classes don't have
# constructors.
no-init,
# TODO(https://github.com/grpc/grpc/issues/261): Doesn't yet play
# nicely with some of our code being implemented in Cython. Maybe in a
# later version?
no-name-in-module,
# TODO(https://github.com/grpc/grpc/issues/261): Suppress these where
# the odd shape of the authentication portion of the API forces them on
# us and enable everywhere else.
protected-access,
# NOTE(nathaniel): Pylint and I will probably never agree on this.
too-few-public-methods,
# NOTE(nathaniel): Pylint and I wil probably never agree on this for
# private classes. For public classes maybe?
too-many-instance-attributes,
# NOTE(nathaniel): Some of our modules have a lot of lines... of
# specification and documentation. Maybe if this were
# lines-of-code-based we would use it.
too-many-lines,
# TODO(https://github.com/grpc/grpc/issues/261): Maybe we could have
# this one if we extracted just a few more helper functions...
too-many-nested-blocks,
# TODO(https://github.com/grpc/grpc/issues/261): Disable unnecessary
# super-init requirement for abstract class implementations for now.
super-init-not-called,
# NOTE(nathaniel): A single statement that always returns program
# control is better than two statements the first of which sometimes
# returns program control and the second of which always returns
# program control. Probably generally, but definitely in the cases of
# if:/else: and for:/else:.
useless-else-on-loop,
no-else-return,
# NOTE(lidiz): Python 3 make object inheritance default, but not PY2
useless-object-inheritance,
# NOTE(lidiz): the import order will be enforced by isort instead
wrong-import-order,
# TODO(https://github.com/PyCQA/pylint/issues/3882): Upgrade Pylint
unsubscriptable-object,
# NOTE(sergiitk): yapf compatibility, ref #25071
bad-continuation,
2 changes: 1 addition & 1 deletion bin/black.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,4 @@ else
fi

# shellcheck disable=SC2086
exec python -m black --config=../../../black.toml ${MODE} .
exec python -m black --config=black.toml ${MODE} .
XuanWang-Amos marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion bin/isort.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,6 @@ fi
# typing is the only module allowed to put imports on the same line:
# https://google.github.io/styleguide/pyguide.html#313-imports-formatting
exec python -m isort "${MODE}" \
--settings-path=../../../black.toml \
--settings-path=black.toml \
XuanWang-Amos marked this conversation as resolved.
Show resolved Hide resolved
framework bin tests

14 changes: 14 additions & 0 deletions black.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[tool.black]
line-length = 80
target-version = [
"py39",
"py310",
"py311",
]

[tool.isort]
profile = "black"
line_length = 80
single_line_exclusions = ["typing"]
force_single_line = true
force_sort_within_sections = true
6 changes: 3 additions & 3 deletions framework/rpc/grpc_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
from grpc_health.v1 import health_pb2_grpc

import framework.rpc
from src.proto.grpc.testing import empty_pb2
from src.proto.grpc.testing import messages_pb2
from src.proto.grpc.testing import test_pb2_grpc
from protos import empty_pb2
from protos import messages_pb2
from protos import test_pb2_grpc

# Type aliases
_LoadBalancerStatsRequest = messages_pb2.LoadBalancerStatsRequest
Expand Down
28 changes: 28 additions & 0 deletions protos/empty.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@

// Copyright 2015 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

package grpc.testing;

// An empty message that you can re-use to avoid defining duplicated empty
// messages in your project. A typical example is to use it as argument or the
// return value of a service API. For instance:
//
// service Foo {
// rpc Bar (grpc.testing.Empty) returns (grpc.testing.Empty) { };
// };
//
message Empty {}
Loading
Loading