feat: ✨ Add validator #108
Conversation
|
Thank you for submitting this pull request! We appreciate your contribution to the project. Before we can merge it, we need to review the changes you've made to ensure they align with our code standards and meet the requirements of the project. We'll get back to you as soon as we can with feedback. Thanks again! |
Reviewer's Guide by SourceryThis PR adds a new validator service to the application, including GitHub workflow updates and API implementation. The validator service is implemented as a Flask application that provides endpoints for validating CWL and CITATION.cff files. Sequence diagram for the CWL file validation processsequenceDiagram
participant User
participant API
participant Validator
User->>API: POST /validate-cwl
API->>Validator: Validate CWL file
Validator-->>API: Validation result
API-->>User: Response with validation result
Sequence diagram for the CITATION.cff file validation processsequenceDiagram
participant User
participant API
participant Validator
User->>API: POST /validate-citation
API->>Validator: Validate CITATION.cff file
Validator-->>API: Validation result
API-->>User: Response with validation result
Class diagram for the new validator APIclassDiagram
class Api {
+title: str
+description: str
+doc: str
}
class Resource {
}
class HelloEverynyan {
+get()
}
class Up {
+get()
}
class ValidateCWL {
+post()
}
class ValidateCitation {
+post()
}
Api <|-- HelloEverynyan
Api <|-- Up
Api <|-- ValidateCWL
Api <|-- ValidateCitation
Resource <|-- HelloEverynyan
Resource <|-- Up
Resource <|-- ValidateCWL
Resource <|-- ValidateCitation
note for Api "This is the main API class for the validator service"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for making updates to your pull request. Our team will take a look and provide feedback as soon as possible. Please wait for any GitHub Actions to complete before editing your pull request. If you have any additional questions or concerns, feel free to let us know. Thank you for your contributions! |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @slugb0t - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| For developer mode: | ||
|
|
||
| ```bash | ||
| python3 app.py --host $HOST --port $PORT |
There was a problem hiding this comment.
issue: The commands shown for developer mode and production mode are identical, which seems incorrect
Consider clarifying the actual differences between running in developer mode versus production mode, as they currently show the same command.
|
|
||
| You will need the following installed on your system: | ||
|
|
||
| - Python 3.8+ |
There was a problem hiding this comment.
issue: Python version requirement is inconsistent in the documentation
The README mentions Python 3.8+ as a requirement, but later specifies Python 3.10 in the Anaconda instructions. Please clarify the actual version requirements.
| return ":)" | ||
|
|
||
| @api.route("/validate-cwl", endpoint="validate-cwl") | ||
| class ValidateCWL(Resource): |
There was a problem hiding this comment.
issue (complexity): Consider extracting common validation and error handling logic into a base class to reduce code duplication.
The validation classes contain significant duplication in their error handling and execution patterns. This can be simplified by extracting common logic into a base class while preserving specific behaviors:
class BaseValidator:
def validate_file_path(self, file_path):
if not file_path:
return {"message": "Validation Error", "error": "file_path is required"}, 400
if ".." in file_path or file_path.startswith("/"):
return {"message": "Validation Error", "error": "Invalid file path"}, 400
if re.search(r'[;&|]', file_path):
return {"message": "Validation Error", "error": "Invalid characters in file path"}, 400
return None
def execute_validation(self, cmd, clean_output_fn=None):
try:
result = subprocess.run(cmd, capture_output=True, text=True, check=True)
stdout = clean_output_fn(result.stdout) if clean_output_fn else result.stdout
stderr = clean_output_fn(result.stderr) if clean_output_fn else result.stderr
return {"message": "valid", "output": stdout, "error": stderr}, 200
except subprocess.CalledProcessError as e:
# Handle error output
return {"message": "invalid", "output": e.stdout, "error": e.stderr}, 400
except Exception as e:
return {"message": "An unexpected error occurred", "error": str(e)}, 500
class ValidateCWL(Resource, BaseValidator):
def post(self):
file_path = api.payload.get('file_path')
validation_error = self.validate_file_path(file_path)
if validation_error:
return validation_error
cmd = ["cwltool", "--validate", file_path]
return self.execute_validation(cmd, clean_output=self.clean_output)
@staticmethod
def clean_output(output):
# Existing clean_output implementationThis refactoring:
- Eliminates duplicate validation code
- Centralizes subprocess execution and error handling
- Allows customization through method overrides
- Makes it easier to add new validators
|
Thanks for closing this pull request! If you have any further questions, please feel free to open a new issue. We are always happy to help! |
Summary by Sourcery
Add a new CWL Validator service with API endpoints for validating CWL and CITATION.cff files, and update deployment workflows to include this service.
New Features:
Enhancements:
Deployment:
Documentation: