- 
                Notifications
    
You must be signed in to change notification settings  - Fork 164
 
          feat(BA-2677): Implement artifact_verifier type plugin in storage-proxy
          #6258
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
94678c8    to
    f232820      
    Compare
  
    6660773    to
    ffecf09      
    Compare
  
    | return ErrorCode( | ||
| domain=ErrorDomain.STORAGE_PROXY, | ||
| operation=ErrorOperation.GENERIC, | ||
| error_detail=ErrorDetail.BAD_REQUEST, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it's correct to set the status code for this error to 400.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements an artifact verifier plugin system for the storage-proxy service to enable verification of downloaded artifacts (e.g., malware scanning). The key changes include:
- Introducing 
AbstractStorageandAbstractStoragePoolinterfaces inai.backend.common.artifact_storagefor shared abstractions - Moving 
ImportStepContextto the common module and removing theprogress_reporterfield - Adding a new 
VERIFYstep in the import pipeline for both Reservoir and HuggingFace artifact sources - Creating a plugin infrastructure for artifact verifiers with 
AbstractArtifactVerifierPlugin - Making 
_resolve_pathmethod public asresolve_pathin VFSStorage - Updating tests to remove 
progress_reporterreferences 
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description | 
|---|---|
| src/ai/backend/common/artifact_storage.py | Introduces shared abstractions for storage interfaces and import context | 
| src/ai/backend/storage/storages/base.py | Removed - AbstractStorage moved to common module | 
| src/ai/backend/storage/storages/vfs_storage.py | Changed _resolve_path to public resolve_path method; updated imports | 
| src/ai/backend/storage/storages/storage_pool.py | Implements AbstractStoragePool interface; updated imports | 
| src/ai/backend/storage/storages/object_storage.py | Updated import path for AbstractStorage | 
| src/ai/backend/storage/services/artifacts/types.py | Removed ImportStepContext (moved to common); added VerifyStepResult; removed Optional import | 
| src/ai/backend/storage/services/artifacts/reservoir.py | Added ReservoirVerifyStep; updated pipeline creation; removed progress_reporter | 
| src/ai/backend/storage/services/artifacts/huggingface.py | Added HuggingFaceVerifyStep; updated pipeline creation; removed progress_reporter | 
| src/ai/backend/storage/services/artifacts/storage_transfer.py | Updated to use public resolve_path method; updated imports | 
| src/ai/backend/storage/context_types.py | New file defining ArtifactVerifierContext for managing verifier plugins | 
| src/ai/backend/storage/plugin.py | Added AbstractArtifactVerifierPlugin and VerificationResult | 
| src/ai/backend/storage/context.py | Added initialization logic for artifact verifier plugins | 
| src/ai/backend/storage/exception.py | Added ArtifactVerifyStorageTypeInvalid and ArtifactVerificationFailedError | 
| src/ai/backend/storage/api/v1/registries/reservoir.py | Passes artifact_verifier_ctx to pipeline creation | 
| src/ai/backend/storage/api/v1/registries/huggingface.py | Passes artifact_verifier_ctx to pipeline creation | 
| src/ai/backend/storage/server.py | Initializes ArtifactVerifierContext in server startup | 
| src/ai/backend/storage/migration.py | Initializes ArtifactVerifierContext for migration context | 
| tests/storage-proxy/storages/test_service.py | Updated import path for AbstractStorage | 
| tests/storage-proxy/artifacts/test_service.py | Removed progress_reporter from test contexts | 
| changes/6258.feat.md | Changelog entry | 
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fb5bf97    to
    308bdfe      
    Compare
  
    
resolves #6230 (BA-2677)
Checklist: (if applicable)