fix: handle prefixed types in validation and use dynamic port allocation#106
fix: handle prefixed types in validation and use dynamic port allocation#106
Conversation
- Add normalization for prefixed type forms (reproschema:X) to full URIs in identify_model_class() for more robust validation - Change HTTP server to use port 0 by default, letting the OS assign an available port instead of hardcoded 8000/4001 - Return actual assigned port from simple_http_server() This fixes test failures that occurred when port 8000 was already in use (e.g., by Docker) causing JSON-LD context loading to fail. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Summary of ChangesHello @yibeichan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the stability and robustness of the system by addressing critical issues related to schema validation and HTTP server port management. It ensures that schema types are correctly interpreted by normalizing prefixed forms to full URIs, and it resolves test failures caused by hardcoded port conflicts by implementing dynamic port allocation for internal HTTP servers. These changes contribute to a more reliable validation process and smoother test execution. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two valuable improvements: handling prefixed types during validation and using dynamic port allocation for the HTTP server to prevent conflicts. The implementation for dynamic port allocation is solid, correctly obtaining and utilizing the OS-assigned port. The logic for normalizing prefixed URIs is also correct. I have one minor suggestion to enhance the robustness of the string replacement logic. Overall, these changes effectively address the stated problems and improve the reliability of the codebase.
| def identify_model_class(category): | ||
| # Normalize prefixed forms to full URIs | ||
| if category.startswith("reproschema:"): | ||
| category = category.replace( |
There was a problem hiding this comment.
Using replace() without a count argument will replace all occurrences of the substring. While the startswith() check makes it likely you only want to replace the prefix, it's safer and clearer to explicitly replace only the first occurrence by specifying count=1. This prevents potential bugs if the category string unexpectedly contains the prefix multiple times.
| category = category.replace( | |
| category = category.replace("reproschema:", "http://schema.repronim.org/", 1) |
There was a problem hiding this comment.
Pull request overview
This PR fixes validation failures that occurred when port 8000 was already in use and handles prefixed type forms in JSON-LD documents more robustly.
Changes:
- Added normalization logic to convert prefixed type forms (
reproschema:X) to full URIs in theidentify_model_class()function - Changed HTTP server functions to use dynamic port allocation (port 0) by default, allowing the OS to assign available ports
- Updated return values to provide the actual assigned port number
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| reproschema/models/utils.py | Added prefix normalization in identify_model_class() to convert reproschema: prefixed types to full http://schema.repronim.org/ URIs |
| reproschema/utils.py | Changed default port from hardcoded values (4001/8000) to 0 for OS-assigned ports; updated simple_http_server() to return actual assigned port; added documentation about dynamic port allocation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
reproschema:X) to full URIs inidentify_model_class()for more robust validationsimple_http_server()Problem
Tests were failing when port 8000 was already in use (e.g., by Docker), causing JSON-LD context loading to fail during validation. The expanded
@typeremained as prefixed forms likereproschema:ResponseActivityinstead of full URIs.Test plan
pytest reproschema/tests/test_validate.py- all 3 tests passpytest reproschema/tests/- 60 passed, 2 skippedRelease Notes
This PR is intended to be part of v1.1.0 release which includes:
loris2reproschemaconverternbdc2reproschemaconverter🤖 Generated with Claude Code