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 public symbols checker script #1816

Merged
merged 30 commits into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
40 changes: 40 additions & 0 deletions .github/workflows/public-api-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: Public API check

on:
pull_request:
types: [opened, synchronize, reopened, labeled, unlabeled]
branches:
- main

jobs:
publicAPICheck:

runs-on: ubuntu-latest

if: "!contains(github.event.pull_request.labels.*.name, 'Skip Public API check')"

steps:
- name: Checkout the repo
uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Checkout main
run: git checkout main

- name: Pull origin
run: git pull --rebase=false origin main

- name: Checkout pull request
run: git checkout ${{ github.event.pull_request.head.sha }}

- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: 3.9

- name: Install tox
run: pip install -U tox-factor

- name: Public API Check
run: tox -e public-symbols-check
5 changes: 3 additions & 2 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ disable=missing-docstring,
exec-used,
super-with-arguments, # temp-pylint-upgrade
isinstance-second-argument-not-valid-type, # temp-pylint-upgrade
raise-missing-from, # temp-pylint-upgrade
unused-argument, # temp-pylint-upgrade
raise-missing-from, # temp-pylint-upgrade
unused-argument, # temp-pylint-upgrade
redefined-builtin,

# Enable the message, report, category or checker with the given id(s). You can
# either give multiple identifier separated by comma (,) or put this option
Expand Down
8 changes: 8 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ You can run:
Python version
- `tox -e lint` to run lint checks on all code

We try to keep the amount of _public symbols_ in our code minimal. A public symbol is any Python identifier that does not start with an underscore.
Every public symbol is something that has to be kept in order to maintain backwards compatibility, so we try to have as few as possible.

To check if your PR is adding public symbols, run `tox -e public-symbols-check`. This will always fail if public symbols are being added. The idea
behind this is that every PR that adds public symbols fails in CI, forcing reviewers to check the symbols to make sure they are strictly necessary.
If after checking them, it is considered that they are indeed necessary, the PR will be labeled with `Skip Public API check` so that this check is not
run.

See
[`tox.ini`](https://github.com/open-telemetry/opentelemetry-python/blob/main/tox.ini)
for more detail on available tox commands.
Expand Down
94 changes: 94 additions & 0 deletions scripts/public_symbols_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# Copyright The OpenTelemetry 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.

from difflib import unified_diff
from pathlib import Path
from re import match
from sys import exit

from git import Repo
from git.db import GitDB

repo = Repo(__file__, odbt=GitDB, search_parent_directories=True)


file_path_symbols = {}


def get_symbols(change_type, diff_lines_getter, prefix):
for diff_lines in (
repo.commit("main")
.diff(repo.head.commit)
.iter_change_type(change_type)
):

b_file_path = diff_lines.b_blob.path

if (
Path(b_file_path).suffix != ".py"
or "opentelemetry" not in b_file_path
):
continue

for diff_line in diff_lines_getter(diff_lines):
matching_line = match(
r"{prefix}({symbol_re})\s=\s.+|"
r"{prefix}def\s({symbol_re})|"
r"{prefix}class\s({symbol_re})".format(
symbol_re=r"[a-zA-Z][_\w]+", prefix=prefix
),
diff_line,
)

if matching_line is not None:
if b_file_path not in file_path_symbols.keys():
file_path_symbols[b_file_path] = []

file_path_symbols[b_file_path].append(
next(filter(bool, matching_line.groups()))
)


def a_diff_lines_getter(diff_lines):
return diff_lines.b_blob.data_stream.read().decode("utf-8").split("\n")


def m_diff_lines_getter(diff_lines):
return unified_diff(
diff_lines.a_blob.data_stream.read().decode("utf-8").split("\n"),
diff_lines.b_blob.data_stream.read().decode("utf-8").split("\n"),
)


get_symbols("A", a_diff_lines_getter, r"")
get_symbols("M", m_diff_lines_getter, r"\+")

if file_path_symbols:
print("The code in this branch adds the following public symbols:")
print()
for file_path, symbols in file_path_symbols.items():
print("- {}".format(file_path))
for symbol in symbols:
print("\t{}".format(symbol))
print()

print(
"Please make sure that all of them are strictly necessary, if not, "
"please consider prefixing them with an underscore to make them "
'private. After that, please label this PR with "Skip Public API '
'check".'
)
exit(1)
else:
print("The code in this branch will not add any public symbols")
12 changes: 11 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ envlist =

; opentelemetry-exporter-jaeger-proto-grpc
py3{6,7,8,9}-test-exporter-jaeger-proto-grpc

; opentelemetry-exporter-jaeger-thrift
py3{6,7,8,9}-test-exporter-jaeger-thrift

Expand Down Expand Up @@ -78,6 +78,7 @@ envlist =
mypy,mypyinstalled
docs
docker-tests
public-symbols-check

[testenv]
deps =
Expand Down Expand Up @@ -186,6 +187,7 @@ deps =
psutil
readme_renderer
httpretty
GitPython

commands_pre =
python -m pip install -e {toxinidir}/opentelemetry-api[test]
Expand Down Expand Up @@ -268,3 +270,11 @@ commands =
pytest {posargs}
commands_post =
docker-compose down -v

[testenv:public-symbols-check]
basepython: python3.8
recreate = True
deps =
GitPython
commands =
python {toxinidir}/scripts/public_symbols_checker.py