Conversation
enAPI spec with 7 endpoints and FOCA integration
Reviewer's GuideAdds a new OpenAPI 3.0 specification for runtime middleware management and wires it into FOCA/Connexion configuration and project docs, without yet implementing controller logic. Sequence diagram for adding a new middleware via the OpenAPI specsequenceDiagram
actor Admin
participant Client as API_client
participant Connexion as FOCA_Connexion
participant Controller as MiddlewaresController
participant DB as MongoDB_middlewares
Admin->>Client: Prepare MiddlewareCreate_payload
Client->>Connexion: POST /ga4gh/tes/v1/middlewares
Connexion->>Connexion: Validate_request_against_OpenAPI
Connexion->>Controller: addMiddleware(MiddlewareCreate)
Controller->>DB: Query existing_middlewares_for_order
DB-->>Controller: Existing_middlewares_with_orders
Controller->>Controller: Compute_final_order_and_shift_stack
Controller->>DB: Insert new_middleware_document
DB-->>Controller: Insert_result_with_id
Controller-->>Connexion: 201 MiddlewareCreateResponse
Connexion-->>Client: HTTP_201_with_JSON_body
Client-->>Admin: Show_created_middleware_id_and_order
Class diagram for middleware management OpenAPI schemasclassDiagram
class MiddlewareConfig {
string _id
string name
string class_path_string
string[] class_path_group
int order
string source
string github_url
object config
boolean enabled
string created_at
string updated_at
}
class MiddlewareCreate {
string name
string class_path_string
string[] class_path_group
int order
string github_url
object config
boolean enabled
}
class MiddlewareUpdate {
string name
int order
object config
boolean enabled
}
class MiddlewareList {
MiddlewareConfig[] middlewares
int total
int limit
int offset
}
class MiddlewareCreateResponse {
string _id
int order
string message
}
class MiddlewareOrder {
string[] ordered_ids
}
class ValidationRequest {
string class_path
string github_url
string code
}
class ValidationErrorItem {
int line
int column
string message
string severity
}
class ValidationWarningItem {
int line
string message
string severity
}
class ValidationResponse {
boolean valid
string message
ValidationErrorItem[] errors
ValidationWarningItem[] warnings
}
class ErrorResponse {
string error
string message
object details
string timestamp
}
MiddlewareList "1" --> "*" MiddlewareConfig : contains
MiddlewareCreateResponse --> MiddlewareConfig : _id_and_order_align_with
MiddlewareOrder "1" --> "*" MiddlewareConfig : orders
ValidationResponse "1" --> "*" ValidationErrorItem : has
ValidationResponse "1" --> "*" ValidationWarningItem : has
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
MiddlewareCreateResponseschema only exposes_id,order, andmessage, but the docs indocs/middleware.mdstate that it returns the full middleware object; consider either expanding the response schema (e.g., embedMiddlewareConfig) or updating the docs to reflect the actual payload to avoid confusion for API consumers. - In
ValidationRequest, theclass_pathfield is described as "Class path or code to validate" while a separatecodefield is also defined; it would be clearer to tighten the descriptions and possibly add explicit rules (e.g., mutual exclusivity or precedence) so clients know exactly how these fields should be used together.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `MiddlewareCreateResponse` schema only exposes `_id`, `order`, and `message`, but the docs in `docs/middleware.md` state that it returns the full middleware object; consider either expanding the response schema (e.g., embed `MiddlewareConfig`) or updating the docs to reflect the actual payload to avoid confusion for API consumers.
- In `ValidationRequest`, the `class_path` field is described as "Class path or code to validate" while a separate `code` field is also defined; it would be clearer to tighten the descriptions and possibly add explicit rules (e.g., mutual exclusivity or precedence) so clients know exactly how these fields should be used together.
## Individual Comments
### Comment 1
<location> `pro_tes/config.yaml:82` </location>
<code_context>
+ - api/middleware_management.yaml
+ add_operation_fields:
+ x-openapi-router-controller: pro_tes.api.middlewares.controllers
+ disable_auth: True
+ connexion:
+ strict_validation: True
</code_context>
<issue_to_address>
**🚨 issue (security):** Exposing middleware management without auth looks risky for production use.
Since this endpoint can change the middleware stack at runtime, keeping `disable_auth: True` lets anyone with network access reorder/enable/disable/add middlewares. Unless this is strictly limited to a trusted/debug environment, this should require authz (or be configurable per deployment) to avoid a straightforward privilege escalation path.
</issue_to_address>
### Comment 2
<location> `pro_tes/api/middleware_management.yaml:572-358` </location>
<code_context>
+ - "507f1f77bcf86cd799439012"
+ - "507f1f77bcf86cd799439013"
+
+ ValidationRequest:
+ type: object
+ description: Request body for validating middleware code
+ required:
+ - class_path
+ properties:
</code_context>
<issue_to_address>
**issue (bug_risk):** ValidationRequest requires class_path even when validating raw code, which conflicts with the description.
The schema contradicts the docs: `code` is documented as an alternative to `class_path`, but `class_path` is required. This blocks use cases where clients submit only raw code or only a `github_url`. Please adjust the schema (e.g., with `oneOf`/`anyOf` to require at least one of `class_path`, `github_url`, or `code`, or by making `class_path` optional and clarifying the validation rules) so the spec matches the intended behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive OpenAPI 3.0 specification for middleware management functionality in proTES, enabling dynamic management of middleware components at runtime. This is the first subtask in a multi-part implementation following a design-first approach, focusing solely on the API specification and documentation without implementing the actual controller logic.
Changes:
- Added complete OpenAPI 3.0 specification defining 7 REST endpoints for middleware CRUD operations, reordering, and validation
- Configured FOCA integration with new middleware management API routes and MongoDB collection
- Added comprehensive documentation describing the API design, endpoints, data models, and implementation plan
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| pro_tes/api/middleware_management.yaml | New OpenAPI 3.0 specification defining middleware management API with 7 endpoints and 9 schemas |
| pro_tes/config.yaml | Added FOCA integration for middleware API, database collection configuration, and routing setup |
| docs/middleware.md | Comprehensive documentation covering API design, implementation details, security considerations, and future work |
Comments suppressed due to low confidence (1)
docs/middleware.md:210
- The changelog date "2026-01-24" is in the future. This should be corrected to use the actual date when this work was completed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| description: | | ||
| Retrieve all configured middlewares with their order, metadata, and status. | ||
| Results are sorted by execution order (ascending) by default. | ||
| operationId: listMiddlewares |
There was a problem hiding this comment.
The operationId naming convention is inconsistent with the existing TES API. The TES API uses PascalCase (GetServiceInfo, CreateTask, GetTask, CancelTask, ListTasks) while the middleware API uses camelCase (listMiddlewares, addMiddleware, getMiddleware, updateMiddleware, deleteMiddleware, reorderMiddlewares, validateMiddleware). For consistency with the existing GA4GH TES API in this codebase, consider using PascalCase for all operationIds in the middleware API.
| local class paths or fetched from GitHub repositories. If order is not specified, | ||
| the middleware is appended to the end of the stack. If order is specified, | ||
| existing middlewares at that position or higher are shifted up by one. | ||
| operationId: addMiddleware |
There was a problem hiding this comment.
The operationId naming convention is inconsistent with the existing TES API. Consider using PascalCase (AddMiddleware instead of addMiddleware) to match the pattern used in the existing API (e.g., CreateTask, GetTask, etc. in pro_tes/api/9e9c5aa.task_execution_service.openapi.yaml).
| get: | ||
| summary: Get middleware details | ||
| description: Retrieve detailed information about a specific middleware by ID | ||
| operationId: getMiddleware |
There was a problem hiding this comment.
The operationId naming convention is inconsistent with the existing TES API. Consider using PascalCase (GetMiddleware instead of getMiddleware) to match the pattern used in the existing API.
| description: | | ||
| Remove a middleware from the execution stack. By default performs soft delete | ||
| (sets enabled=false). Use force=true query parameter for hard deletion. | ||
| operationId: deleteMiddleware |
There was a problem hiding this comment.
The operationId naming convention is inconsistent with the existing TES API. Consider using PascalCase (DeleteMiddleware instead of deleteMiddleware) to match the pattern used in the existing API.
| ValidationRequest: | ||
| type: object | ||
| description: Request body for validating middleware code | ||
| required: | ||
| - class_path | ||
| properties: | ||
| class_path: | ||
| type: string | ||
| description: Class path or code to validate | ||
| example: "pro_tes.plugins.middlewares.task_distribution.distance.TaskDistributionDistance" | ||
| github_url: | ||
| type: string | ||
| description: GitHub URL for fetching middleware code | ||
| nullable: true | ||
| pattern: '^https://raw\.githubusercontent\.com/.+\.py$' | ||
| example: "https://raw.githubusercontent.com/user/repo/main/middleware.py" | ||
| code: | ||
| type: string | ||
| description: Raw Python code to validate (alternative to class_path) | ||
| nullable: true |
There was a problem hiding this comment.
The ValidationRequest schema marks 'class_path' as required but also provides 'code' as an "alternative to class_path" (line 590). This is contradictory - if 'code' is truly an alternative, then 'class_path' should not be required. Consider using oneOf to express that either 'class_path' or 'code' is required, but not both, or clarify the validation logic in the description.
| middlewares: | ||
| indexes: | ||
| - keys: | ||
| name: 1 |
There was a problem hiding this comment.
The database index is created on the 'name' field, but the OpenAPI spec indicates that both 'name' and 'class_path' should be unique (based on the 400 error description mentioning 'duplicate name/class_path'). Consider adding indexes on both 'name' and 'class_path' fields with unique constraints to enforce this at the database level, or clarify whether only 'name' needs to be unique.
| name: 1 | |
| name: 1 | |
| options: | |
| "unique": True | |
| - keys: | |
| class_path: 1 | |
| options: | |
| "unique": True |
| ## File Structure | ||
|
|
||
| ``` | ||
| pro_tes/ | ||
| ├── api/ | ||
| │ └── middleware_management.yaml (OpenAPI specification) | ||
| └── config.yaml (FOCA integration) | ||
|
|
||
| docs/ | ||
| └── middleware.md (This file) | ||
| ``` | ||
|
|
||
| ## Documentation Deliverables | ||
|
|
||
| **API Documentation**: Comprehensive guide with request/response examples for each endpoint. Includes curl commands, common use cases, and troubleshooting tips. | ||
|
|
||
| **Architecture Decision Record**: Documents twelve major design decisions with rationale, alternatives considered, and consequences. Serves as reference for future development. | ||
|
|
||
| **Postman Collection**: Ready-to-use collection with fourteen pre-configured requests. Includes environment variables, test scripts, and example data for all scenarios. | ||
|
|
||
| **Quick Reference**: Single-page reference with essential endpoints, parameters, and response codes. Designed for daily development use. | ||
|
|
||
| **Validation Script**: Bash script that validates OpenAPI syntax using multiple tools. Checks for common errors like undefined schema references and invalid endpoint definitions. | ||
|
|
||
| ## Testing Approach | ||
|
|
||
| This subtask focuses on specification validation rather than runtime testing since no executable code is implemented yet. Validation performed: | ||
|
|
||
| **YAML Syntax**: Verified file parses correctly as valid YAML without syntax errors. | ||
|
|
||
| **OpenAPI Compliance**: Confirmed specification follows OpenAPI 3.0 standards including required fields, valid schema definitions, and proper reference resolution. | ||
|
|
There was a problem hiding this comment.
The documentation references several files that are not included in this PR: 'docs/api/middleware_management.md', 'middleware_management.postman_collection.json', 'QUICK_REFERENCE.md', 'docs/architecture/middleware_api_design.md', and 'scripts/validate_openapi.sh'. Either these files should be included in this PR, or the documentation should clarify that these are planned deliverables for future PRs. The "Documentation Deliverables" section also describes these files in detail, which creates confusion about what is actually delivered in this subtask.
| ErrorResponse: | ||
| type: object | ||
| description: Standard error response | ||
| required: | ||
| - error | ||
| - message | ||
| properties: | ||
| error: | ||
| type: string | ||
| description: Error type/code | ||
| example: "MiddlewareNotFound" | ||
| message: | ||
| type: string | ||
| description: Human-readable error message | ||
| example: "Middleware with ID '507f1f77bcf86cd799439011' not found" | ||
| details: | ||
| type: object | ||
| description: Additional error details | ||
| nullable: true | ||
| additionalProperties: true | ||
| timestamp: | ||
| type: string | ||
| format: date-time | ||
| description: Error timestamp | ||
| example: "2026-01-24T10:30:00Z" |
There was a problem hiding this comment.
The ErrorResponse schema is inconsistent with the existing error handling convention. The FOCA exception configuration (pro_tes/config.yaml:120) requires error responses to have 'message' and 'code' fields, but the ErrorResponse schema defines 'error', 'message', 'details', and 'timestamp' fields without a 'code' field. For consistency with the existing error handling (see pro_tes/exceptions.py:52-121), the ErrorResponse schema should include a 'code' field containing the HTTP status code, and consider using 'code' instead of or in addition to 'error' for the error type.
pro_tes/config.yaml
Outdated
| - "pro_tes.plugins.middlewares.task_distribution.random.TaskDistributionRandom" | ||
|
No newline at end of file |
There was a problem hiding this comment.
Trailing whitespace on empty line. Remove the trailing spaces for consistency with code style.
| - "pro_tes.plugins.middlewares.task_distribution.random.TaskDistributionRandom" | |
| - "pro_tes.plugins.middlewares.task_distribution.random.TaskDistributionRandom" |
docs/middleware.md
Outdated
| ```yaml | ||
| specs: | ||
| - path: | ||
| - api/middleware_management.yaml | ||
| add_operation_fields: | ||
| x-openapi-router-controller: pro_tes.api.middlewares.controllers | ||
| disable_auth: True |
There was a problem hiding this comment.
The FOCA configuration for the middleware management API is setting disable_auth: True, which exposes all middleware management endpoints without any authentication. In any non-isolated environment this allows an unauthenticated attacker to list, create, modify, or delete middlewares, potentially leading to arbitrary code execution or unauthorized changes to workflow behavior. Require authentication for these endpoints (e.g., by wiring them into your existing security scheme instead of using disable_auth) and, if needed, restrict unauthenticated access to development-only configs that are not deployed to production.
keshxvdayal
force-pushed
the
feature/middleware-api-spec
branch
from
b3118e8 to
5025c12
Compare
uniqueg
left a comment
uniqueg
left a comment
There was a problem hiding this comment.
I didn't have a chance to review everything yet, but please go ahead and address the existing comments.
| url: https://www.apache.org/licenses/LICENSE-2.0.html | ||
|
|
||
| servers: | ||
| - url: /ga4gh/tes/v1 |
There was a problem hiding this comment.
consider hosting these elsewhere, given that these are not GA4GH (TES) endpoints. Just /protes/v1 would be fine, I guess.
| - name: limit | ||
| in: query | ||
| description: Maximum number of results to return | ||
| required: false | ||
| schema: | ||
| type: integer | ||
| minimum: 1 | ||
| maximum: 100 | ||
| default: 50 | ||
| - name: offset | ||
| in: query | ||
| description: Number of results to skip (for pagination) | ||
| required: false | ||
| schema: | ||
| type: integer | ||
| minimum: 0 | ||
| default: 0 |
There was a problem hiding this comment.
Please double check to ensure that pagination is in line with the GA4GH pagination guide that was recently published: https://github.com/ga4gh/TASC/blob/main/recommendations/API%20pagination%20guide.md
| summary: Add local middleware | ||
| value: | ||
| name: "Distance-based Router" | ||
| class_path: "pro_tes.plugins.middlewares.task_distribution.distance.TaskDistributionDistance" |
There was a problem hiding this comment.
Note that this only works if the package containing the class is installed. If middlewares are shipped within proTES (which is a bit of an antipattern and will probably not be supported in the future), this will be fine - but if not, I think we need to come up with something better.
Possibly we need more than one parameter, (1) a package root path (containing a setup.py or pyproject.toml file that will allow installation) OR package name (to be installed from PyPI or another registry) OR Git repo AND (2) a class path, as an entry point to ensure that we know what code to execute in the package once it has been installed. This also makes the use of different sources (local, Git, package registry) consistent.
| - "pro_tes.plugins.middlewares.task_distribution.distance.TaskDistributionDistance" | ||
| - "pro_tes.plugins.middlewares.task_distribution.random.TaskDistributionRandom" |
There was a problem hiding this comment.
What about combinations of local, GitHub etc. middlewares. Not all middlewares need to come from the same source, so how is the mapping done? I think the structuring of the model needs to be different - each one will need a URI (local path, GitHub URL, package registry), an entry point (class path) and possibly a type (local, Git, package registry). See comment above for more details.
uniqueg
left a comment
uniqueg
left a comment
There was a problem hiding this comment.
Still some ways to go, but we're getting there! :)
docs/middleware.md
Outdated
| The Middleware Management API is defined using OpenAPI 3.0 specification and provides seven REST endpoints for complete middleware lifecycle management. | ||
|
|
||
| ### API Endpoints | ||
|
|
||
| **List Middlewares** - GET /protes/v1/middlewares | ||
| Returns all configured middlewares with pagination and filtering support. Results are sorted by execution order by default. Supports filtering by enabled status and source type. | ||
|
|
||
| **Add Middleware** - POST /protes/v1/middlewares | ||
| Creates a new middleware in the execution stack. Supports loading from local packages, GitHub repositories, or PyPI packages. Automatically handles order assignment and stack shifting. | ||
|
|
||
| **Get Middleware Details** - GET /protes/v1/middlewares/{middleware_id} | ||
| Retrieves detailed information about a specific middleware including configuration, metadata, and execution statistics. | ||
|
|
||
| **Update Middleware** - PUT /protes/v1/middlewares/{middleware_id} | ||
| Updates middleware configuration. Only allows modification of name, order, config parameters, and enabled status. Package path and entry point cannot be changed for security reasons. | ||
|
|
||
| **Delete Middleware** - DELETE /protes/v1/middlewares/{middleware_id} | ||
| Removes a middleware from the stack. Supports soft delete (disable) by default and hard delete with force parameter. | ||
|
|
||
| **Reorder Stack** - PUT /protes/v1/middlewares/reorder | ||
| Reorders the entire middleware execution stack by accepting an ordered array of middleware IDs. | ||
|
|
||
| **Validate Code** - POST /protes/v1/middlewares/validate | ||
| Validates middleware code before creation. Checks Python syntax, required interface implementation, and security constraints. | ||
|
|
||
| ### Data Model | ||
|
|
||
| The API uses comprehensive schema definitions to structure request and response data: | ||
|
|
||
| **MiddlewareConfig**: Complete middleware representation including ID, name, package information (source type, package path, entry point), execution order, enabled status, configuration parameters, and timestamps. | ||
|
|
||
| **MiddlewareCreate**: Request body for creating new middleware. Includes name, package source configuration (local path, GitHub URL, or PyPI package), entry point (class path), optional order, enabled flag, and configuration dict. | ||
|
|
||
| **MiddlewareUpdate**: Request body for updates. Limited to name, order, config, and enabled fields to prevent unauthorized code changes. | ||
|
|
||
| **MiddlewareList**: Paginated list response containing middleware array, total count, page information, and navigation tokens following GA4GH pagination guidelines. | ||
|
|
||
| **MiddlewareCreateResponse**: Response after successful creation including the middleware ID, assigned order, and success message. | ||
|
|
||
| **MiddlewareOrder**: Request body for reordering containing an array of middleware IDs in desired execution order. | ||
|
|
||
| **ValidationRequest**: Code validation request containing package source information and entry point to validate. | ||
|
|
||
| **ValidationResponse**: Validation result including validity boolean, validation messages, error details with line numbers, and warnings. | ||
|
|
||
| **ErrorResponse**: Standard error response with HTTP status code, error message, and optional details. |
There was a problem hiding this comment.
You can replace all of this with just a reference to the Swagger Editor, with something like this: https://editor.swagger.io/?url=https://raw.githubusercontent.com/elixir-cloud-aai/proTES/refs/heads/feature/middleware-api-spec/pro_tes/api/middleware_management.yaml
The documentation is much more comprehensive, easier to parse and play with, and always up to date. Obviously you need to adapt the link to point to the path it will have once the PR is merged, not the one from this branch.
docs/middleware.md
Outdated
|
|
||
| **Order-Based Execution**: Middlewares execute in ascending order. Lower order values run first. This provides clear, predictable execution flow that's easy to understand and debug. | ||
|
|
||
| **Fallback Group Support**: Allows grouping multiple middleware sources in a single middleware entry. If the first middleware fails, the system automatically tries the next one in the list. Each middleware in a fallback group specifies its own source, package path, and entry point. |
There was a problem hiding this comment.
Clarify: Not the next one in the last, but the next one in the fallback stack/group.
docs/middleware.md
Outdated
|
|
||
| **Fallback Group Support**: Allows grouping multiple middleware sources in a single middleware entry. If the first middleware fails, the system automatically tries the next one in the list. Each middleware in a fallback group specifies its own source, package path, and entry point. | ||
|
|
||
| **Soft Delete Default**: DELETE operations disable rather than remove middlewares by default. This preserves execution history and allows easy rollback. Hard delete requires explicit force parameter. |
There was a problem hiding this comment.
This is an interesting idea, but I'm not sure this implementation although it raises some questions.
First of all, it breaks with REST semantics. DELETE operations delete a resource. If you want to modify a resource, you should use UPDATE (or possibly PUT), e.g., on a enabled parameter in the data model. This would also make it easy to reactivate it (no new endpoint needed).
Also, can this be applied to an entire fallback group at once?
And how do you actually roll back the entire state - if reproducibility is your problem here?
In any case, how do you ensure reproducibility in another proTES instance?
To be honest, there are so many questions around this that I think this should be disabled for now. Rather, you should think about how you can capture enough information about the middleware stack (put a whole copy in it, including versions) in the task logs that it can be recreated in another instance (or the same one in the future).
Speaking of versions: How can different versions of middlewares be handled? Say I want to update a repo from Git to a new commit/tag? Or a new PyPI version? And how are multiple versions handled? Do you plan to sandbox/isolate the installation artifacts or dependencies for each middleware (you absolutely should).
This brings me to another question? How does DELETE work? Does it just remove the middleware from the stack, or does it also undo any installations? Because over time, conflicts may accumulate, if isolation is not in place. And even if it is, this grows over time...
Of course, most of these points are discussion points for the next PR. For the API model in this PR, I would just remove any provisioning for soft deletion.
|
|
||
| **ErrorResponse**: Standard error response with HTTP status code, error message, and optional details. | ||
|
|
||
| ### Key Features |
There was a problem hiding this comment.
None of these are currently available, so this is something that should make the docs only once these features are actually implemented. You really need to understand and internalize the continuous documentation philosophy. It doesn't matter if you think it's there next week, tomorrow or even in 2 hours. Each PR should update the documentation to represent the state of that code change - you never know what will happen.
There was a problem hiding this comment.
Given that this PR does not have any controllers (which is okay, it shouldn't have), none of these features are currently supported and should, in principle, not be in the docs until the next PR. Same goes for anything else that depends on the controllers being implemented and functional.
docs/middleware.md
Outdated
| **Immutable Package Configuration**: Once created, a middleware's package source and entry point cannot be changed. This prevents security risks from code substitution attacks. To change implementation, users must delete and recreate. | ||
|
|
||
| **Multiple Package Sources**: Supports loading middleware from: | ||
| - **Local packages**: Installed Python packages with a class path entry point |
There was a problem hiding this comment.
This is actually discouraged and should be deprecated, to be removed in the future (once the middlewares inside proTES are migrated to a new repo). The reason being that it's difficult/impossible to reproduce. Plus, in most cases, users won't have access to the running instance's file system, so it's really just useful for developers and local deployments.
For now, please note that it's for development purposes and is deprecated, and list it as the third, not the first option.
| description: Complete middleware configuration object | ||
| required: | ||
| - _id | ||
| - name |
There was a problem hiding this comment.
Maybe shouldn't be required? You could make it nullable and use the package or repo name etc. as default if the user doesn't supply one.
| - _id | ||
| - name | ||
| - source | ||
| - order |
There was a problem hiding this comment.
This should also be nullable. Make it easy for people to use the API.
| - created_at | ||
| - updated_at |
There was a problem hiding this comment.
These should be set by the system, so should be nullable as well - a user shouldn't supply these.
But now I'm realizing why you require all of these - you are using the model in the request AND response?
Check out the readOnly and writeOnly types in OpenAPI. With these, you can give more fine-grained control over what you expect from the user in the request and what you need in the response. In any case, this may not completely solve the problem. If you want to always respond with a param, just implement it like that. But don't require it from the user if it's not absolutely necessary.
| order: | ||
| type: integer | ||
| description: Execution order (0 = first) | ||
| minimum: 0 |
| source: | ||
| $ref: '#/components/schemas/MiddlewareSource' | ||
|
|
||
| ValidationResponse: |
There was a problem hiding this comment.
I cannot reason about this without seeing the implementation (or implementation plan). Please remove it from this PR and readd when you implement it. Same with the Validation Request and endpoint. In fact, leave this out of the next PR, too. It's a bonus feature and does not have high priority at this point.
uniqueg
changed the title
|
|
||
| The API includes comprehensive security controls: | ||
|
|
||
| **Authentication Required**: All middleware management endpoints require authentication through the existing proTES security scheme. |
| ## Dependencies | ||
|
|
||
| **External**: | ||
| - OpenAPI 3.0 specification format | ||
| - FOCA framework (Flask-based configuration) | ||
| - Connexion (OpenAPI request routing) | ||
| - MongoDB (persistence layer) | ||
|
|
||
| **Internal**: | ||
| - Existing proTES API structure | ||
| - Current middleware plugin architecture | ||
| - MongoDB database configuration |
| ## API Specification Location | ||
|
|
||
| The complete OpenAPI 3.0 specification is available at: `pro_tes/api/middleware_management.yaml` |
There was a problem hiding this comment.
This means: Remove it here. Not addressed.
| - name: enabled | ||
| in: query | ||
| description: Filter by enabled status | ||
| required: false | ||
| schema: | ||
| type: boolean |
2 participants
Detailed Implementation Plan
Create OpenAPI Specification Document
data/specs/middleware_management.yaml/ga4gh/tes/v1/middlewares)GET /middlewares- List all configured middlewaresPOST /middlewares- Add new middleware (local or from GitHub)GET /middlewares/{middleware_id}- Get middleware detailsPUT /middlewares/{middleware_id}- Update middleware configurationDELETE /middlewares/{middleware_id}- Remove middlewarePUT /middlewares/reorder- Reorder middleware stackPOST /middlewares/validate- Validate middleware code without addingMiddlewareConfig- Full middleware configuration objectMiddlewareCreate- Request body for creating middlewareMiddlewareUpdate- Request body for updating middlewareMiddlewareList- Array response with pagination supportMiddlewareOrder- Order configuration for reorderingValidationRequest- Code validation requestErrorResponse- Standard error formatIntegrate with FOCA Configuration
pro_tes/config.yamlto reference the new specWrite API Documentation
docs/api/middleware_management.mdValidation and Review
openapi-generator validateSummary by Sourcery
Add an OpenAPI specification and configuration wiring for a new runtime middleware management API in proTES.
New Features:
Enhancements:
Documentation: