Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Jan 18, 2025

Add Hub client implementation for accessing hub routes

Changes:

  • Add Hub client class in vlmrun/client/hub.py
  • Implement get_health(), list_domains(), and get_schema() methods
  • Add comprehensive test coverage in tests/test_hub.py
  • Update deprecated hub methods in client.py to use new client

The implementation follows the existing SDK patterns and includes:

  • Type-safe request/response models
  • Proper error handling
  • Comprehensive test coverage for all endpoints
  • Documentation with usage examples

Implements these hub routes:

  • GET /health
  • GET /domains
  • POST /schema

All tests passing locally.

Link to Devin run: https://app.devin.ai/sessions/d3e36b9912304dc09775fb7b2f0f4e0a

- Add Hub client for accessing hub routes
- Implement get_health, list_domains, get_schema methods
- Add comprehensive test coverage
- Update deprecated hub methods to use new client

Co-Authored-By: Sudeep Pillai <[email protected]>
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

assert response["status"] == "ok"
assert response["hub_version"] == "1.0.0"

# Test convenience method
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this call and the method


def get_hub_version(self):
raise NotImplementedError("Hub version not yet implemented")
def get_hub_version(self) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this comvenience method

"""
return self.hub.get_health()["hub_version"]

# Deprecated methods - use hub resource instead
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this convenience method

"""Use client.hub.list_domains() instead. This method is deprecated."""
return self.hub.list_domains()

def submit_hub_item(self, path: str, name: str, version: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this convenience method

from vlmrun.client.base_requestor import APIRequestor, APIError


class HubSchemaQueryRequest:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use pydantic BaseModel to define these classes instead

return {"domain": self.domain}


class HubSchemaQueryResponse:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use pydantic BaseModel to define these classes instead under types.py

@devin-ai-integration
Copy link
Contributor Author

Devin is currently unreachable - the session may have died.

4 similar comments
@devin-ai-integration
Copy link
Contributor Author

Devin is currently unreachable - the session may have died.

@devin-ai-integration
Copy link
Contributor Author

Devin is currently unreachable - the session may have died.

@devin-ai-integration
Copy link
Contributor Author

Devin is currently unreachable - the session may have died.

@devin-ai-integration
Copy link
Contributor Author

Devin is currently unreachable - the session may have died.

… types.py per PR feedback

Co-Authored-By: Sudeep Pillai <[email protected]>
assert kwargs["url"] == "/hub/health"
return {"status": "ok", "hub_version": "1.0.0"}, 200, {}

client = Client(api_key="test-key")
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this into a pytest fixture that can be reused.

def get_hub_version(self):
raise NotImplementedError("Hub version not yet implemented")

# Deprecated methods - use hub resource instead
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove these methods, and instead expect the user to only use client.hub.

"""Use client.hub.list_domains() instead. This method is deprecated."""
return self.hub.list_domains()

def submit_hub_item(self, path: str, name: str, version: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this

raise NotImplementedError("Hub version not yet implemented")

# Deprecated methods - use hub resource instead
def list_hub_items(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this

This client provides access to the hub routes for managing schemas and domains.
"""

def __init__(self, client) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use type annotations for the client

"""
self._client = client

def get_health(self) -> Dict[str, Any]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Return a pydantic schema instead. Define the HubHealthResponse in vlmrun/common/types.py

except Exception as e:
raise APIError(f"Failed to check hub health: {str(e)}")

def list_domains(self) -> List[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Return pydantic types

- Remove deprecated hub methods from client.py
- Convert hub models to Pydantic BaseModel
- Add proper type annotations and safety checks
- Update tests to use new models

Co-Authored-By: Sudeep Pillai <[email protected]>
@spillai spillai merged commit 5a26357 into main Jan 22, 2025
3 checks passed
@spillai spillai deleted the devin/1737181931-add-hub-client branch January 23, 2025 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants