[core]python: support pfvs in python #5930
Conversation
jerry-024
force-pushed
the
support_pvfs_python
branch
from
3b792ca to
ddc23d1
Compare
| for table_name in self.rest_api.list_tables(database_name): | ||
| self.rest_api.drop_table(Identifier.create(database_name, table_name)) | ||
| self.rest_api.drop_database(database_name) | ||
| return True |
There was a problem hiding this comment.
Here recursive drop database is not right. Rest api drop database will just delete database, should check table existence when recursive is false. We can take RESTCatalog.dropDatabase as reference.
| for table_name in self.rest_api.list_tables(database_name): | ||
| self.rest_api.drop_table(Identifier.create(database_name, table_name)) | ||
| self.rest_api.drop_database(database_name) | ||
| return True |
There was a problem hiding this comment.
It seems rmdir should only support delete empty dir. Can we make a double check of its behavior?
| self._create_object_table(pvfs_identifier) | ||
| except AlreadyExistsException: | ||
| pass | ||
| table = self.rest_api.get_table(table_identifier) |
There was a problem hiding this comment.
Maybe we can optimize by first get_table, and then create if table not exist, in order to reduce api calls in most cases.
| f"created is not supported for path: {path}" | ||
| ) | ||
| elif isinstance(pvfs_identifier, PVFSDatabaseIdentifier): | ||
| return self.rest_api.get_database(pvfs_identifier.name).created_at |
There was a problem hiding this comment.
According to https://filesystem-spec.readthedocs.io/en/latest/api.html, created should return datetime.datetime, created_at is a long value in millisecond.
| elif isinstance(pvfs_identifier, PVFSTableIdentifier): | ||
| table_identifier = Identifier.create(pvfs_identifier.database, pvfs_identifier.name) | ||
| if pvfs_identifier.sub_path is None: | ||
| return self.rest_api.get_table(table_identifier).created_at |
There was a problem hiding this comment.
Should convert to datetime.datetime.
| f"modified is not supported for path: {path}" | ||
| ) | ||
| elif isinstance(pvfs_identifier, PVFSDatabaseIdentifier): | ||
| return self.rest_api.get_database(pvfs_identifier.name).updated_at |
There was a problem hiding this comment.
Should convert to datetime.datetime.
| elif isinstance(pvfs_identifier, PVFSTableIdentifier): | ||
| table_identifier = Identifier.create(pvfs_identifier.database, pvfs_identifier.name) | ||
| if pvfs_identifier.sub_path is None: | ||
| return self.rest_api.get_table(table_identifier).updated_at |
There was a problem hiding this comment.
Should convert to datetime.datetime.
|
It is better to set user agent. In Java SDK, we set RESTCatalogOptions.HTTP_USER_AGENT as HadoopPVFS. So here maybe we can use PythonPVFS? |
Purpose
support pfvs in python: implements fsspec
Tests
API and Format
Documentation