damccorm commented on code in PR #23476:
URL: https://github.com/apache/beam/pull/23476#discussion_r995844971
##########
.github/workflows/playground_examples_reusable.yml:
##########
@@ -27,14 +27,73 @@ on:
subdirs:
type: string
required: true
+ whitelist:
Review Comment:
Nit: Can we use `allowlist` instead of `whitelist`? There's a general
industry move towards allowlist for
[clarity](https://www.linkedin.com/pulse/allowlist-blocklist-better-terms-everyone-lets-use-them-rob-black/)
and
[inclusiveness](https://www.ncsc.gov.uk/blog-post/terminology-its-not-black-and-white).
##########
playground/infrastructure/checker.py:
##########
@@ -16,45 +16,91 @@
"""
Module implements check to define if it is needed to run CI step for Beam
Playground examples
+
+Returns exit code 1 if no examples found
+
+All paths are relative to BEAM_ROOT_DIR
"""
+import argparse
+import logging
import os
import sys
+from pathlib import PurePath
+from typing import List
+from api.v1.api_pb2 import Sdk
from config import Config
from helper import get_tag
-root_dir = os.getenv("BEAM_ROOT_DIR")
-
-if root_dir is None:
- raise KeyError(
- "BEAM_ROOT_DIR environment variable should be specified in os")
-
-
-def check(paths) -> bool:
- pathsArr = []
- startInd = 0
- lastInd = 0
- while lastInd < len(paths):
- if paths[lastInd] == ".":
- lastInd += 1
- while lastInd < len(paths) and paths[lastInd] != " ":
- lastInd += 1
- pathsArr.append(paths[startInd:lastInd])
- lastInd += 1
- startInd = lastInd
- lastInd += 1
- for filepath in pathsArr:
- extension = filepath.split(os.extsep)[-1]
- if extension not in Config.SDK_TO_EXTENSION.values():
+
+def parse_args() -> argparse.Namespace:
+ parser = argparse.ArgumentParser()
+ parser.add_argument("--sdk", type=str, choices=Sdk.keys())
+ parser.add_argument(
+ "--whitelist",
+ nargs="*",
+ default=[],
+ required=True,
+ type=PurePath,
+ help="if any path falls in here, return success",
+ )
+ parser.add_argument(
+ "--paths",
+ nargs="*",
+ default=[],
+ required=True,
+ type=PurePath,
+ help="paths to check",
+ )
+ parser.add_argument("-v", "--verbose", action="store_true")
+ return parser.parse_args()
+
+
+def check_in_whitelist(paths: List[PurePath], whitelist: List[PurePath]) ->
bool:
Review Comment:
Same thing here, please prefer allowlist
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
