-
Notifications
You must be signed in to change notification settings - Fork 2
Adam/epd 2068 update python sdk for self hosted deployments (#360) #362
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
Closed
josorio-autoblocks
wants to merge
1
commit into
main
from
jose/epd-2072-update-sdks-to-support-default-value-and-add-safe-parse
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| from autoblocks._impl.util import AutoblocksEnvVar | ||
|
|
||
| API_ENDPOINT = "https://api.autoblocks.ai" | ||
| API_ENDPOINT_V2 = "https://api-v2.autoblocks.ai" | ||
| API_ENDPOINT_V2 = AutoblocksEnvVar.V2_API_ENDPOINT_V2.get() or "https://api-v2.autoblocks.ai" | ||
| INGESTION_ENDPOINT = "https://ingest-event.autoblocks.ai" | ||
| REVISION_LATEST = "latest" | ||
| REVISION_UNDEPLOYED = "undeployed" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| from pydantic import model_validator | ||
|
|
||
| from autoblocks._impl.datasets.models.schema import SchemaProperty | ||
| from autoblocks._impl.datasets.models.schema import create_schema_property | ||
| from autoblocks._impl.util import cuid_generator | ||
|
|
||
| # Type alias for schema property lists | ||
|
|
@@ -35,6 +36,30 @@ class Dataset(BaseModel): | |
| extra="allow", | ||
| ) | ||
|
|
||
| @field_validator("schema_properties", mode="before") | ||
| @classmethod | ||
| def validate_schema_properties(cls, v: Optional[List[Dict[str, Any]]]) -> Optional[List[SchemaProperty]]: | ||
| """Validate and convert schema properties using factory function""" | ||
| if v is None: | ||
| return None | ||
|
|
||
| if not isinstance(v, list): | ||
| return v | ||
|
|
||
| result = [] | ||
| for item in v: | ||
| if isinstance(item, dict): | ||
| try: | ||
| result.append(create_schema_property(item)) | ||
| except (ValueError, TypeError): | ||
| # If we can't parse a specific property, skip it rather than failing entirely | ||
| # This provides better resilience against API changes | ||
| continue | ||
| else: | ||
| result.append(item) | ||
|
|
||
| return result | ||
|
Comment on lines
+39
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Duplicate validation logic between Dataset and DatasetSchema classes. Consider extracting to a shared validator function to maintain DRY principles |
||
|
|
||
|
|
||
| class DatasetSchema(BaseModel): | ||
| """Dataset schema V2""" | ||
|
|
@@ -51,6 +76,30 @@ class DatasetSchema(BaseModel): | |
| extra="allow", | ||
| ) | ||
|
|
||
| @field_validator("schema_properties", mode="before") | ||
| @classmethod | ||
| def validate_schema_properties(cls, v: Optional[List[Dict[str, Any]]]) -> Optional[List[SchemaProperty]]: | ||
| """Validate and convert schema properties using factory function""" | ||
| if v is None: | ||
| return None | ||
|
|
||
| if not isinstance(v, list): | ||
| return v | ||
|
|
||
| result = [] | ||
| for item in v: | ||
| if isinstance(item, dict): | ||
| try: | ||
| result.append(create_schema_property(item)) | ||
| except (ValueError, TypeError): | ||
| # If we can't parse a specific property, skip it rather than failing entirely | ||
| # This provides better resilience against API changes | ||
| continue | ||
| else: | ||
| result.append(item) | ||
|
|
||
| return result | ||
|
|
||
|
|
||
| class DatasetItem(BaseModel): | ||
| """Dataset item V2""" | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,203 @@ | ||
| """Tests for AutoblocksAppClient dataset deserialization with defaultValue fields.""" | ||
|
|
||
| from autoblocks._impl.config.constants import API_ENDPOINT_V2 | ||
| from autoblocks.api.app_client import AutoblocksAppClient | ||
|
|
||
|
|
||
| def test_dataset_deserialization_with_default_value(httpx_mock): | ||
| """Test that datasets with defaultValue field in schema properties can be deserialized correctly.""" | ||
| # This test reproduces the exact error scenario from the user's issue | ||
| httpx_mock.add_response( | ||
| url=f"{API_ENDPOINT_V2}/apps/test-app/datasets", | ||
| method="GET", | ||
| status_code=200, | ||
| json=[ | ||
| { | ||
| "id": "dataset-id-1", | ||
| "externalId": "test-dataset", | ||
| "name": "Test Dataset", | ||
| "createdAt": "2023-01-01T00:00:00Z", | ||
| "latestRevisionId": "revision-1", | ||
| "schemaVersion": 1, | ||
| "schema": [ | ||
| { | ||
| "id": "string-prop-id", | ||
| "name": "String Property", | ||
| "type": "String", | ||
| "required": True, | ||
| }, | ||
| { | ||
| "id": "s23p00ugn0gb5zhmg123", | ||
| "name": "Number Property", | ||
| "type": "Number", | ||
| "required": False, | ||
| "defaultValue": 30, # This was causing the validation error | ||
| }, | ||
| { | ||
| "id": "select-prop-id", | ||
| "name": "Select Property", | ||
| "type": "Select", | ||
| "required": False, | ||
| "options": ["option1", "option2"], | ||
| "defaultValue": "option1", | ||
| }, | ||
| ], | ||
| } | ||
| ], | ||
| match_headers={"Authorization": "Bearer mock-api-key"}, | ||
| ) | ||
|
|
||
| client = AutoblocksAppClient(app_slug="test-app", api_key="mock-api-key") | ||
| datasets = client.datasets.list() | ||
|
|
||
| # Verify the dataset was deserialized successfully | ||
| assert len(datasets) == 1 | ||
| dataset = datasets[0] | ||
|
|
||
| assert dataset.id == "dataset-id-1" | ||
| assert dataset.external_id == "test-dataset" | ||
| assert dataset.name == "Test Dataset" | ||
| assert dataset.schema_version == 1 | ||
|
|
||
| # Verify schema properties were parsed correctly | ||
| assert dataset.schema_properties is not None | ||
| assert len(dataset.schema_properties) == 3 | ||
|
|
||
| # Check string property | ||
| string_prop = dataset.schema_properties[0] | ||
| assert string_prop.id == "string-prop-id" | ||
| assert string_prop.name == "String Property" | ||
| assert string_prop.type.value == "String" | ||
| assert string_prop.required is True | ||
| assert string_prop.default_value is None | ||
|
|
||
| # Check number property with defaultValue | ||
| number_prop = dataset.schema_properties[1] | ||
| assert number_prop.id == "s23p00ugn0gb5zhmg123" | ||
| assert number_prop.name == "Number Property" | ||
| assert number_prop.type.value == "Number" | ||
| assert number_prop.required is False | ||
| assert number_prop.default_value == 30 | ||
|
|
||
| # Check select property with defaultValue | ||
| select_prop = dataset.schema_properties[2] | ||
| assert select_prop.id == "select-prop-id" | ||
| assert select_prop.name == "Select Property" | ||
| assert select_prop.type.value == "Select" | ||
| assert select_prop.required is False | ||
| assert select_prop.default_value == "option1" | ||
| # Type check to ensure we have a SelectProperty before accessing options | ||
| from autoblocks._impl.datasets.models.schema import SelectProperty | ||
|
|
||
| assert isinstance(select_prop, SelectProperty) | ||
| assert select_prop.options == ["option1", "option2"] | ||
|
|
||
|
|
||
| def test_dataset_deserialization_with_invalid_schema_property(): | ||
| """Test that datasets with invalid schema properties are handled gracefully.""" | ||
| from autoblocks._impl.api.utils.serialization import deserialize_model | ||
| from autoblocks._impl.datasets.models.dataset import Dataset | ||
|
|
||
| # Mock data with an invalid schema property that should be skipped | ||
| dataset_data = { | ||
| "id": "dataset-id-1", | ||
| "externalId": "test-dataset", | ||
| "name": "Test Dataset", | ||
| "schema": [ | ||
| { | ||
| "id": "valid-prop-id", | ||
| "name": "Valid Property", | ||
| "type": "String", | ||
| "required": True, | ||
| }, | ||
| { | ||
| "id": "invalid-prop-id", | ||
| "name": "Invalid Property", | ||
| "type": "UnknownType", # This should cause the property to be skipped | ||
| "required": False, | ||
| }, | ||
| { | ||
| "id": "another-valid-prop-id", | ||
| "name": "Another Valid Property", | ||
| "type": "Number", | ||
| "required": False, | ||
| "defaultValue": 42, | ||
| }, | ||
| ], | ||
| } | ||
|
|
||
| # This should not raise an exception | ||
| dataset = deserialize_model(Dataset, dataset_data) | ||
|
|
||
| # Verify the dataset was created successfully | ||
| assert dataset.id == "dataset-id-1" | ||
| assert dataset.external_id == "test-dataset" | ||
| assert dataset.name == "Test Dataset" | ||
|
|
||
| # Should have only 2 valid properties (invalid one skipped) | ||
| assert dataset.schema_properties is not None | ||
| assert len(dataset.schema_properties) == 2 | ||
|
|
||
| # Check that valid properties are present | ||
| prop_names = [prop.name for prop in dataset.schema_properties] | ||
| assert "Valid Property" in prop_names | ||
| assert "Another Valid Property" in prop_names | ||
| assert "Invalid Property" not in prop_names | ||
|
|
||
|
|
||
| def test_dataset_schema_property_factory_function(): | ||
| """Test the schema property factory function directly.""" | ||
| from autoblocks._impl.datasets.models.schema import SchemaPropertyType | ||
| from autoblocks._impl.datasets.models.schema import SelectProperty | ||
| from autoblocks._impl.datasets.models.schema import create_schema_property | ||
|
|
||
| # Test creating a number property with defaultValue | ||
| number_prop_data = { | ||
| "id": "test-number-prop", | ||
| "name": "Test Number", | ||
| "type": "Number", | ||
| "required": False, | ||
| "defaultValue": 42, | ||
| } | ||
|
|
||
| number_prop = create_schema_property(number_prop_data) | ||
| assert number_prop.id == "test-number-prop" | ||
| assert number_prop.name == "Test Number" | ||
| assert number_prop.type == SchemaPropertyType.NUMBER | ||
| assert number_prop.required is False | ||
| assert number_prop.default_value == 42 | ||
|
|
||
| # Test creating a select property with defaultValue | ||
| select_prop_data = { | ||
| "id": "test-select-prop", | ||
| "name": "Test Select", | ||
| "type": "Select", | ||
| "required": True, | ||
| "options": ["option1", "option2", "option3"], | ||
| "defaultValue": "option2", | ||
| } | ||
|
|
||
| select_prop = create_schema_property(select_prop_data) | ||
| assert select_prop.id == "test-select-prop" | ||
| assert select_prop.name == "Test Select" | ||
| assert select_prop.type == SchemaPropertyType.SELECT | ||
| assert select_prop.required is True | ||
| assert select_prop.default_value == "option2" | ||
| # Type check before accessing options | ||
| assert isinstance(select_prop, SelectProperty) | ||
| assert select_prop.options == ["option1", "option2", "option3"] | ||
|
|
||
| # Test creating a string property without defaultValue | ||
| string_prop_data = { | ||
| "id": "test-string-prop", | ||
| "name": "Test String", | ||
| "type": "String", | ||
| "required": True, | ||
| } | ||
|
|
||
| string_prop = create_schema_property(string_prop_data) | ||
| assert string_prop.id == "test-string-prop" | ||
| assert string_prop.name == "Test String" | ||
| assert string_prop.type == SchemaPropertyType.STRING | ||
| assert string_prop.required is True | ||
| assert string_prop.default_value is None |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
logic: Consider what happens if
get()returns an empty string - would still fallback to default URL as intended?