Skip to content

Commit

Permalink
[#2929] improvement(pyClient): Optimize Python client code and add in…
Browse files Browse the repository at this point in the history
…tegration test (#3125)

### What changes were proposed in this pull request?

1. Add `_` before Python class member variable name, let it to private
access level.
2. Add Metalake, Schema E2E integration test.

### Why are the changes needed?

Fix: #2929

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

CI Passed.

Co-authored-by: Xun Liu <[email protected]>
Co-authored-by: yuhui <[email protected]>
  • Loading branch information
3 people authored Apr 23, 2024
1 parent b265be7 commit 4db1a3c
Show file tree
Hide file tree
Showing 45 changed files with 837 additions and 553 deletions.
10 changes: 2 additions & 8 deletions clients/client-python/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,10 @@
```bash
pip install -e '.[dev]'
```

2. Run tests
```bash
cd gravitino
./gradlew :clients:client-python:test
```

3. Run integration tests
2. Run integration tests
```bash
cd gravitino
./gradlew compileDistribution -x test
./gradlew :clients:client-python:integrationTest
./gradlew :clients:client-python:test
```
26 changes: 13 additions & 13 deletions clients/client-python/gravitino/api/catalog_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ class RenameCatalog:
def __init__(self, new_name):
self.new_name = new_name

def get_new_name(self):
def new_name(self):
"""Retrieves the new name set for the catalog.
Returns:
The new name of the catalog.
"""
return self.new_name

def __eq__(self, other):
def __eq__(self, other) -> bool:
"""Compares this RenameCatalog instance with another object for equality. Two instances are
considered equal if they designate the same new name for the catalog.
Expand Down Expand Up @@ -112,15 +112,15 @@ class UpdateCatalogComment:
def __init__(self, new_comment):
self.new_comment = new_comment

def get_new_comment(self):
def new_comment(self):
"""Retrieves the new comment intended for the catalog.
Returns:
The new comment that has been set for the catalog.
"""
return self.new_comment

def __eq__(self, other):
def __eq__(self, other) -> bool:
"""Compares this UpdateCatalogComment instance with another object for equality.
Two instances are considered equal if they designate the same new comment for the catalog.
Expand Down Expand Up @@ -159,23 +159,23 @@ def __init__(self, property, value):
self.property = property
self.value = value

def get_property(self):
def property(self):
"""Retrieves the name of the property being set in the catalog.
Returns:
The name of the property.
"""
return self.property

def get_value(self):
def value(self):
"""Retrieves the value assigned to the property in the catalog.
Returns:
The value of the property.
"""
return self.value

def __eq__(self, other):
def __eq__(self, other) -> bool:
"""Compares this SetProperty instance with another object for equality.
Two instances are considered equal if they have the same property and value for the catalog.
Expand Down Expand Up @@ -211,17 +211,17 @@ class RemoveProperty:
"""A catalog change to remove a property from the catalog."""

def __init__(self, property):
self.property = property
self._property = property

def get_property(self):
"""Retrieves the name of the property to be removed from the catalog.
Returns:
The name of the property for removal.
"""
return self.property
return self._property

def __eq__(self, other):
def __eq__(self, other) -> bool:
"""Compares this RemoveProperty instance with another object for equality.
Two instances are considered equal if they target the same property for removal from the catalog.
Expand All @@ -233,7 +233,7 @@ def __eq__(self, other):
"""
if not isinstance(other, CatalogChange.RemoveProperty):
return False
return self.property == other.property
return self._property == other._property

def __hash__(self):
"""Generates a hash code for this RemoveProperty instance.
Expand All @@ -242,7 +242,7 @@ def __hash__(self):
Returns:
A hash code value for this property removal operation.
"""
return hash(self.property)
return hash(self._property)

def __str__(self):
"""Provides a string representation of the RemoveProperty instance.
Expand All @@ -251,4 +251,4 @@ def __str__(self):
Returns:
A string summary of the property removal operation.
"""
return f"REMOVEPROPERTY {self.property}"
return f"REMOVEPROPERTY {self._property}"
2 changes: 1 addition & 1 deletion clients/client-python/gravitino/api/fileset.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


class Fileset(Auditable):
"""An interface representing a fileset under a schema {@link Namespace}. A fileset is a virtual
"""An interface representing a fileset under a schema Namespace. A fileset is a virtual
concept of the file or directory that is managed by Gravitino. Users can create a fileset object
to manage the non-tabular data on the FS-like storage. The typical use case is to manage the
training data for AI workloads. The major difference compare to the relational table is that the
Expand Down
76 changes: 44 additions & 32 deletions clients/client-python/gravitino/api/fileset_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
This software is licensed under the Apache License version 2.
"""
from abc import ABC
from dataclasses import field

from dataclasses_json import config


class FilesetChange(ABC):
Expand Down Expand Up @@ -62,16 +65,18 @@ def remove_property(property):
class RenameFileset:
"""A fileset change to rename the fileset."""

_new_name: str = field(metadata=config(field_name='new_name'))

def __init__(self, new_name):
self.new_name = new_name
self._new_name = new_name

def get_new_name(self):
def new_name(self):
"""Retrieves the new name set for the fileset.
Returns:
The new name of the fileset.
"""
return self.new_name
return self._new_name

def __eq__(self, other):
"""Compares this RenameFileset instance with another object for equality.
Expand All @@ -85,7 +90,7 @@ def __eq__(self, other):
"""
if not isinstance(other, FilesetChange.RenameFileset):
return False
return self.new_name == other.new_name
return self._new_name == other.new_name()

def __hash__(self):
"""Generates a hash code for this RenameFileset instance.
Expand All @@ -94,7 +99,7 @@ def __hash__(self):
Returns:
A hash code value for this renaming operation.
"""
return hash(self.new_name)
return hash(self._new_name)

def __str__(self):
"""Provides a string representation of the RenameFile instance.
Expand All @@ -103,23 +108,25 @@ def __str__(self):
Returns:
A string summary of this renaming operation.
"""
return f"RENAMEFILESET {self.new_name}"
return f"RENAMEFILESET {self._new_name}"

class UpdateFilesetComment:
"""A fileset change to update the fileset comment."""

_new_comment: str = field(metadata=config(field_name='new_comment'))

def __init__(self, new_comment):
self.new_comment = new_comment
self._new_comment = new_comment

def get_new_comment(self):
def new_comment(self):
"""Retrieves the new comment intended for the fileset.
Returns:
The new comment that has been set for the fileset.
"""
return self.new_comment
return self._new_comment

def __eq__(self, other):
def __eq__(self, other) -> bool:
"""Compares this UpdateFilesetComment instance with another object for equality.
Two instances are considered equal if they designate the same new comment for the fileset.
Expand All @@ -131,7 +138,7 @@ def __eq__(self, other):
"""
if not isinstance(other, FilesetChange.UpdateFilesetComment):
return False
return self.new_comment == other.new_comment
return self._new_comment == other.new_comment()

def __hash__(self):
"""Generates a hash code for this UpdateFileComment instance.
Expand All @@ -140,7 +147,7 @@ def __hash__(self):
Returns:
A hash code representing this comment update operation.
"""
return hash(self.new_comment)
return hash(self._new_comment)

def __str__(self):
"""Provides a string representation of the UpdateFilesetComment instance.
Expand All @@ -149,32 +156,35 @@ def __str__(self):
Returns:
A string summary of this comment update operation.
"""
return f"UPDATEFILESETCOMMENT {self.new_comment}"
return f"UPDATEFILESETCOMMENT {self._new_comment}"

class SetProperty:
"""A fileset change to set the property and value for the fileset."""

def __init__(self, property, value):
self.property = property
self.value = value
_property: str = field(metadata=config(field_name='property'))
_value: str = field(metadata=config(field_name='value'))

def get_property(self):
def __init__(self, property: str, value: str):
self._property = property
self._value = value

def property(self):
"""Retrieves the name of the property being set in the fileset.
Returns:
The name of the property.
"""
return self.property
return self._property

def get_value(self):
def value(self):
"""Retrieves the value assigned to the property in the fileset.
Returns:
The value of the property.
"""
return self.value
return self._value

def __eq__(self, other):
def __eq__(self, other) -> bool:
"""Compares this SetProperty instance with another object for equality.
Two instances are considered equal if they have the same property and value for the fileset.
Expand All @@ -186,7 +196,7 @@ def __eq__(self, other):
"""
if not isinstance(other, FilesetChange.SetProperty):
return False
return self.property == other.property and self.value == other.value
return self._property == other.property() and self._value == other.value()

def __hash__(self):
"""Generates a hash code for this SetProperty instance.
Expand All @@ -195,7 +205,7 @@ def __hash__(self):
Returns:
A hash code value for this property setting.
"""
return hash((self.property, self.value))
return hash((self._property, self._value))

def __str__(self):
"""Provides a string representation of the SetProperty instance.
Expand All @@ -204,23 +214,25 @@ def __str__(self):
Returns:
A string summary of the property setting.
"""
return f"SETPROPERTY {self.property} {self.value}"
return f"SETPROPERTY {self._property} {self._value}"

class RemoveProperty:
"""A fileset change to remove a property from the fileset."""

def __init__(self, property):
self.property = property
_property: str = field(metadata=config(field_name='property'))

def get_property(self):
def __init__(self, property: str):
self._property = property

def property(self):
"""Retrieves the name of the property to be removed from the fileset.
Returns:
The name of the property for removal.
"""
return self.property
return self._property

def __eq__(self, other):
def __eq__(self, other) -> bool:
"""Compares this RemoveProperty instance with another object for equality.
Two instances are considered equal if they target the same property for removal from the fileset.
Expand All @@ -232,7 +244,7 @@ def __eq__(self, other):
"""
if not isinstance(other, FilesetChange.RemoveProperty):
return False
return self.property == other.property
return self._property == other.property()

def __hash__(self):
"""Generates a hash code for this RemoveProperty instance.
Expand All @@ -241,7 +253,7 @@ def __hash__(self):
Returns:
A hash code value for this property removal operation.
"""
return hash(self.property)
return hash(self._property)

def __str__(self):
"""Provides a string representation of the RemoveProperty instance.
Expand All @@ -250,4 +262,4 @@ def __str__(self):
Returns:
A string summary of the property removal operation.
"""
return f"REMOVEPROPERTY {self.property}"
return f"REMOVEPROPERTY {self._property}"
44 changes: 44 additions & 0 deletions clients/client-python/gravitino/api/metalake.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
"""
Copyright 2024 Datastrato Pvt Ltd.
This software is licensed under the Apache License version 2.
"""
from abc import abstractmethod
from typing import Optional, Dict

from gravitino.api.auditable import Auditable


class Metalake(Auditable):
"""
The interface of a metalake. The metalake is the top level entity in the gravitino system,
containing a set of catalogs.
"""

@abstractmethod
def name(self) -> str:
"""The name of the metalake.
Returns:
str: The name of the metalake.
"""
pass

@abstractmethod
def comment(self) -> Optional[str]:
"""The comment of the metalake. Note. this method will return None if the comment is not set for
this metalake.
Returns:
Optional[str]: The comment of the metalake.
"""
pass

@abstractmethod
def properties(self) -> Optional[Dict[str, str]]:
"""The properties of the metalake. Note, this method will return None if the properties are not
set.
Returns:
Optional[Dict[str, str]]: The properties of the metalake.
"""
pass
Loading

0 comments on commit 4db1a3c

Please sign in to comment.