HonahX commented on code in PR #2557:
URL: https://github.com/apache/polaris/pull/2557#discussion_r2346997553
##########
client/python/generate_clients.py:
##########
@@ -266,7 +272,30 @@ def prepend_licenses() -> None:
logger.info("License fix complete.")
+def prepare_spec_dir():
+ logger.info("Preparing spec directory...")
+ spec_dir = Path(SPEC_DIR)
+
+ if spec_dir.exists():
+ logger.info("Spec directory already exists.")
+ return
+
+ # Create the spec directory if it doesn't exist
+ spec_source_dir = CLIENT_DIR.parent.parent / "spec"
+ if spec_source_dir.exists():
+ logger.info(f"Copying spec directory from {spec_source_dir} to
{spec_dir}")
+ shutil.copytree(spec_source_dir, spec_dir)
+ logger.info("Spec directory copied.")
+ else:
+ # This will be hit during an sdist build if spec directory wasn't in
the package.
+ logger.error(
+ "Fatal: spec directory is missing and the source to copy it from
was not found."
+ )
+ sys.exit(1)
Review Comment:
```suggestion
def prepare_spec_dir():
logger.info("Preparing spec directory...")
spec_dir = Path(SPEC_DIR)
spec_source_dir = CLIENT_DIR.parent.parent / "spec"
if not spec_source_dir.exists:
logger.error(
"Fatal: spec directory is missing and the source to copy it from
was not found."
)
sys.exit(1)
if spec_dir.exists():
logger.info("Spec directory already exists.")
source_yaml_files = set(str(f.relative_to(spec_source_dir)) for f in
spec_source_dir.rglob("*.yaml"))
target_yaml_files = set(str(f.relative_to(spec_dir)) for f in
spec_dir.rglob("*.yaml"))
missing_files = source_yaml_files - target_yaml_files
if missing_files:
logger.info(f"Missing YAML files in spec directory: {',
'.join(missing_files)}, updating spec directory")
else:
return
# Create the spec directory if it doesn't exist
logger.info(f"Copying spec directory from {spec_source_dir} to
{spec_dir}")
shutil.copytree(spec_source_dir, spec_dir, dirs_exist_ok=True)
logger.info("Spec directory copied.")
```
Shall we add the check to verify that the existing spec file contains all
the yaml files in the source dir?
I feel like we could even just simply copy and overwrite specs every time to
ensure the spec files are always the latest. WDYT?
##########
client/python/generate_clients.py:
##########
@@ -83,6 +83,8 @@
Path("generate_clients.py"),
Path(".venv"),
Path("dist/"),
+ Path("templates/"),
+ Path("PKG-INFO"),
]
Review Comment:
Shall we add `Path("spec/")` to the exclude list too? All the spec we copied
here should already have the license header.
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
