Skip to content

Add filtering option for listing sandboxes#235

Merged
jakubno merged 4 commits intomainfrom
implement-server-side-sandbox-metadata-lookup-e2b-1421
Jan 17, 2025
Merged

Add filtering option for listing sandboxes#235
jakubno merged 4 commits intomainfrom
implement-server-side-sandbox-metadata-lookup-e2b-1421

Conversation

@jakubno
Copy link
Copy Markdown
Member

@jakubno jakubno commented Jan 10, 2025

Description

Allow users to filter out sandbox based on metadata

Description by Callstackai

This PR introduces a filtering option for listing sandboxes based on metadata, allowing users to specify query parameters to filter results.

Diagrams of code changes
sequenceDiagram
    Client->>+API: GET /sandboxes?query=user%3Dabc%26app%3Dprod
    API->>API: Parse query parameter
    API->>API: URL unescape query string
    API->>Orchestrator: GetSandboxes()
    Orchestrator-->>API: Return all sandboxes
    
    alt Has query parameter
        API->>API: Parse filters into key-value pairs
        API->>API: Filter sandboxes based on metadata
        API->>API: Keep only sandboxes matching all filters
    end
    
    API-->>-Client: Return filtered sandboxes
Loading
Files Changed
FileSummary
packages/api/internal/api/api.gen.goUpdated the GetSandboxes method signature to accept filtering parameters.
packages/api/internal/api/spec.gen.goNo changes made.
packages/api/internal/api/types.gen.goAdded a new type GetSandboxesParams to define parameters for the GetSandboxes method.
packages/api/internal/handlers/sandboxes_list.goImplemented filtering logic in the GetSandboxes method based on query parameters.
spec/openapi.ymlUpdated OpenAPI specification to include the new query parameter for filtering sandboxes.

This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions .yml. See list of supported languages.

@jakubno jakubno requested a review from ValentaTomas as a code owner January 10, 2025 13:10
@linear
Copy link
Copy Markdown

linear bot commented Jan 10, 2025

Copy link
Copy Markdown
Contributor

@callstackai callstackai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key Issues

The current implementation is inefficient as it iterates over instanceInfo multiple times for each filter key, which could significantly impact performance on large datasets; consider optimizing by combining filter checks into a single pass.

Comment thread packages/api/internal/handlers/sandboxes_list.go Outdated
Comment thread spec/openapi.yml
Co-authored-by: callstackai[bot] <186726322+callstackai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@callstackai callstackai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key Issues

The use of nested loops results in inefficient O(n*m) time complexity, which could significantly impact performance on large datasets.

Comment thread packages/api/internal/handlers/sandboxes_list.go Outdated
@jakubno jakubno added the improvement Improvement for current functionality label Jan 10, 2025
@ValentaTomas ValentaTomas added feature New feature and removed improvement Improvement for current functionality labels Jan 13, 2025
@ValentaTomas
Copy link
Copy Markdown
Member

ValentaTomas commented Jan 13, 2025

The next steps here:

  • Change naming from filter to query (or similar)
  • Ensure the keys and values are escaped
  • Ensure we are ready to expand the querying later (make ANDs explicit?)

@jakubno
Copy link
Copy Markdown
Member Author

jakubno commented Jan 16, 2025

Everything should be ready

@jakubno jakubno merged commit 1abd4f4 into main Jan 17, 2025
@jakubno jakubno deleted the implement-server-side-sandbox-metadata-lookup-e2b-1421 branch January 17, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants