Skip to content

Commit df1f62e

Browse files
authored
chore: add dedicated Python CI + ruff (#274)
<!-- Please use this template for your pull request. --> <!-- Please use the sections that you need and delete other sections --> ## This PR <!-- add the description of the PR here --> - added a dedicated Python CI workflow - added ruff for linting (only dependency sorting is enabled for now) and formatting - added a couple of missing type hints, enabling mypy comes in a separate PR Signed-off-by: gruebel <[email protected]>
1 parent 807c0d2 commit df1f62e

File tree

5 files changed

+103
-43
lines changed

5 files changed

+103
-43
lines changed

.github/workflows/pr-python.yaml

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
name: PR Python checks
2+
3+
on:
4+
pull_request:
5+
branches:
6+
- main
7+
8+
permissions:
9+
contents: read
10+
11+
env:
12+
WORKING_DIR: tools/repo_parser
13+
14+
jobs:
15+
lint:
16+
runs-on: ubuntu-latest
17+
steps:
18+
- uses: actions/checkout@v4
19+
- uses: actions/setup-python@v5
20+
with:
21+
python-version: 3.12
22+
23+
- name: Install dependencies
24+
working-directory: ${{ env.WORKING_DIR }}
25+
run: pip install -r requirements.txt
26+
27+
- name: Format
28+
working-directory: ${{ env.WORKING_DIR }}
29+
run: ruff format --check
30+
- name: Lint
31+
working-directory: ${{ env.WORKING_DIR }}
32+
run: ruff check

tools/repo_parser/pyproject.toml

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
[tool.ruff]
2+
line-length = 120
3+
target-version = "py312"
4+
5+
[tool.ruff.lint]
6+
select = [
7+
"I",
8+
]
9+
10+
[tool.ruff.format]
11+
quote-style = "single"

tools/repo_parser/requirements.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
mypy
22
pytest
33
pytest-cov
4+
ruff

tools/repo_parser/spec_finder.py

+47-37
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,32 @@
11
#!/usr/bin/python
2-
import urllib.request
2+
from __future__ import annotations
3+
4+
import configparser
35
import json
4-
import re
5-
import difflib
66
import os
7+
import re
78
import sys
8-
import configparser
9-
from typing import TypedDict, Optional, cast
9+
import urllib.request
10+
from typing import Any, TypedDict, cast
11+
1012

1113
class Config(TypedDict):
1214
file_extension: str
13-
multiline_regex: Optional[str]
14-
number_subregex: Optional[str]
15-
text_subregex: Optional[str]
16-
inline_comment_prefix: Optional[str]
15+
multiline_regex: str | None
16+
number_subregex: str | None
17+
text_subregex: str | None
18+
inline_comment_prefix: str | None
1719

18-
def _demarkdown(t):
20+
21+
def _demarkdown(t: str) -> str:
1922
return t.replace('**', '').replace('`', '').replace('"', '')
2023

21-
def get_spec_parser(code_dir) -> Config:
24+
25+
def get_spec_parser(code_dir: str) -> Config:
2226
with open(os.path.join(code_dir, '.specrc')) as f:
2327
data = '\n'.join(f.readlines())
2428

25-
typical = configparser.ConfigParser(comment_prefixes=None)
29+
typical = configparser.ConfigParser(comment_prefixes=())
2630
typical.read_string(data)
2731
retval = typical['spec']
2832

@@ -40,17 +44,18 @@ def get_spec_parser(code_dir) -> Config:
4044
return cast(Config, retval)
4145

4246

43-
44-
def get_spec(force_refresh=False, path_prefix="./"):
47+
def get_spec(force_refresh: bool = False, path_prefix: str = './') -> dict[str, Any]:
4548
spec_path = os.path.join(path_prefix, 'specification.json')
46-
print("Going to look in ", spec_path)
47-
data = ""
49+
print('Going to look in ', spec_path)
50+
data = ''
4851
if os.path.exists(spec_path) and not force_refresh:
4952
with open(spec_path) as f:
5053
data = ''.join(f.readlines())
5154
else:
5255
# TODO: Status code check
53-
spec_response = urllib.request.urlopen('https://raw.githubusercontent.com/open-feature/spec/main/specification.json')
56+
spec_response = urllib.request.urlopen(
57+
'https://raw.githubusercontent.com/open-feature/spec/main/specification.json'
58+
)
5459
raw = []
5560
for i in spec_response.readlines():
5661
raw.append(i.decode('utf-8'))
@@ -59,7 +64,8 @@ def get_spec(force_refresh=False, path_prefix="./"):
5964
f.write(data)
6065
return json.loads(data)
6166

62-
def specmap_from_file(actual_spec):
67+
68+
def specmap_from_file(actual_spec: dict[str, Any]) -> dict[str, str]:
6369
spec_map = {}
6470
for entry in actual_spec['rules']:
6571
number = re.search(r'[\d.]+', entry['id']).group()
@@ -73,12 +79,13 @@ def specmap_from_file(actual_spec):
7379
spec_map[number] = _demarkdown(ch['content'])
7480
return spec_map
7581

76-
def find_covered_specs(config, data):
82+
83+
def find_covered_specs(config: Config, data: str) -> dict[str, dict[str, str]]:
7784
repo_specs = {}
7885
for match in re.findall(config['multiline_regex'], data, re.MULTILINE | re.DOTALL):
7986
match = match.replace('\n', '').replace(config['inline_comment_prefix'], '')
8087
# normalize whitespace
81-
match = re.sub(" {2,}", " ", match.strip())
88+
match = re.sub(' {2,}', ' ', match.strip())
8289
number = re.findall(config['number_subregex'], match)[0]
8390

8491
text_with_concat_chars = re.findall(config['text_subregex'], match, re.MULTILINE | re.DOTALL)
@@ -94,13 +101,13 @@ def find_covered_specs(config, data):
94101
print(f"Skipping {match} b/c we couldn't parse it")
95102
return repo_specs
96103

97-
def gen_report(from_spec, from_repo):
104+
105+
def gen_report(from_spec: dict[str, str], from_repo: dict[str, dict[str, str]]) -> dict[str, set[str]]:
98106
extra = set()
99-
missing = set()
100107
different_text = set()
101108
good = set()
102109

103-
missing = set(from_spec.keys()) # assume they're all missing
110+
missing = set(from_spec.keys()) # assume they're all missing
104111

105112
for number, text in from_repo.items():
106113
if number in missing:
@@ -121,7 +128,13 @@ def gen_report(from_spec, from_repo):
121128
}
122129

123130

124-
def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directory=None, json_report=False):
131+
def main(
132+
refresh_spec: bool = False,
133+
diff_output: bool = False,
134+
limit_numbers: str | None = None,
135+
code_directory: str | None = None,
136+
json_report: bool = False,
137+
) -> None:
125138
report = {
126139
'extra': set(),
127140
'missing': set(),
@@ -134,12 +147,11 @@ def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directo
134147

135148
spec_map = specmap_from_file(actual_spec)
136149

137-
138150
repo_specs = {}
139151
missing = set(spec_map.keys())
140152
bad_num = 0
141153

142-
for root, dirs, files in os.walk(".", topdown=False):
154+
for root, dirs, files in os.walk('.', topdown=False):
143155
for name in files:
144156
F = os.path.join(root, name)
145157
if ('.%s' % config['file_extension']) not in name:
@@ -153,25 +165,24 @@ def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directo
153165

154166
for number in report['different-text']:
155167
bad_num += 1
156-
print(f"{number} is bad.")
168+
print(f'{number} is bad.')
157169
if diff_output:
158-
print("Official:")
159-
print("\t%s" % spec_map[number])
160-
print("")
161-
print("Ours:")
162-
print("\t%s" % repo_specs[number])
170+
print('Official:')
171+
print('\t%s' % spec_map[number])
172+
print('')
173+
print('Ours:')
174+
print('\t%s' % repo_specs[number])
163175

164176
bad_num += len(report['extra'])
165177
for number in report['extra']:
166178
print(f"{number} is defined in our tests, but couldn't find it in the spec")
167179

168-
169180
missing = report['missing']
170181
bad_num += len(missing)
171182
if len(missing) > 0:
172183
print('In the spec, but not in our tests: ')
173184
for m in sorted(missing):
174-
print(f" {m}: {spec_map[m]}")
185+
print(f' {m}: {spec_map[m]}')
175186

176187
if json_report:
177188
for k in report.keys():
@@ -190,9 +201,8 @@ def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directo
190201
parser.add_argument('--refresh-spec', action='store_true', help='Re-downloads the spec')
191202
parser.add_argument('--diff-output', action='store_true', help='print the text differences')
192203
parser.add_argument('--code-directory', action='store', required=True, help='directory to find code in')
193-
parser.add_argument('--json-report', action='store_true', help="Store a json report into ${extension}-report.json")
194-
parser.add_argument('specific_numbers', metavar='num', type=str, nargs='*',
195-
help='limit this to specific numbers')
204+
parser.add_argument('--json-report', action='store_true', help='Store a json report into ${extension}-report.json')
205+
parser.add_argument('specific_numbers', metavar='num', type=str, nargs='*', help='limit this to specific numbers')
196206

197207
args = parser.parse_args()
198208
main(

tools/repo_parser/test_spec_finder.py

+12-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import re
21
from spec_finder import find_covered_specs, gen_report
32

3+
44
def test_simple_singleline():
55
text = """
66
// spec:4.3.6:The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.:end
@@ -13,7 +13,10 @@ def test_simple_singleline():
1313
}
1414
output = find_covered_specs(cfg, text)
1515
assert '4.3.6' in output
16-
assert output['4.3.6']['text'] == "The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value."
16+
assert (
17+
output['4.3.6']['text']
18+
== 'The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.'
19+
)
1720

1821

1922
def test_multiline_comment():
@@ -31,20 +34,23 @@ def test_multiline_comment():
3134
}
3235
output = find_covered_specs(cfg, text)
3336
assert '4.3.7' in output
34-
assert output['4.3.7']['text'] == """The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value."""
37+
assert (
38+
output['4.3.7']['text']
39+
== """The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value."""
40+
)
3541

3642

3743
def test_report():
3844
spec = {
39-
'1.2.3': "good text",
45+
'1.2.3': 'good text',
4046
'2.3.4': 'different text',
41-
'3.4.5': 'missing'
47+
'3.4.5': 'missing',
4248
}
4349

4450
repo = {
4551
'1.2.3': 'good text',
4652
'2.3.4': 'it is different',
47-
'4.5.6': 'extra'
53+
'4.5.6': 'extra',
4854
}
4955

5056
report = gen_report(spec, repo)

0 commit comments

Comments
 (0)