XiaoHongbo-Hope commented on code in PR #922:
URL: https://github.com/apache/paimon-rust/pull/922#discussion_r4091903464
##########
crates/paimon/src/table/rest_env.rs:
##########
@@ -336,6 +383,33 @@ impl RESTEnv {
}
}
+fn response_identifier(
+ requested: &Identifier,
+ response: &crate::api::GetTableResponse,
+) -> Result<Identifier> {
+ let database = response
+ .database
+ .as_deref()
+ .unwrap_or_else(|| requested.database());
+ let name = response.name.as_deref().ok_or_else(|| Error::DataInvalid {
+ message: format!("Table response for database '{database}' missing
name"),
+ source: None,
+ })?;
+ let identifier = Identifier::new(database, name);
+ identifier.validate()?;
+ if &identifier != requested {
Review Comment:
Fixed in apache/paimon#10123 commit `372b2e5ef7`: PyPaimon now passes
`get_object_name()`, and its REST test server returns the requested
branch-qualified name while still looking up base-table metadata. Added the
response-contract assertion and ran the real #922 binding branch cases: 2
passed.
##########
bindings/python/src/table.rs:
##########
@@ -83,6 +84,34 @@ impl PyTable {
Ok(Self::new(Arc::new(table)))
}
+ /// Reuse the matching REST table response and merged catalog options.
+ /// Skips config/get-table requests, preserving REST snapshots and token
refresh.
+ #[staticmethod]
+ #[pyo3(signature = (response_json, *, database, table, rest_options))]
Review Comment:
Fixed in apache/paimon#10123 commit `372b2e5ef7`: the caller and unit test
now use `rest_options=`. The native REST integration tests also reject any
`PaimonCatalog` reload when `Table.from_rest_response` is available. I ran them
against the real #922 extension: 14 passed.
##########
bindings/python/src/table.rs:
##########
@@ -83,6 +84,34 @@ impl PyTable {
Ok(Self::new(Arc::new(table)))
}
+ /// Reuse the matching REST table response and merged catalog options.
+ /// Skips config/get-table requests, preserving REST snapshots and token
refresh.
+ #[staticmethod]
+ #[pyo3(signature = (response_json, *, database, table, rest_options))]
Review Comment:
Fixed in apache/paimon#10123 commit `372b2e5ef7`: the caller and unit test
now use `rest_options=`. The native REST integration tests also reject any
`PaimonCatalog` reload when `Table.from_rest_response` is available. I ran them
against the real #922 extension: 14 passed.
##########
crates/paimon/src/table/rest_env.rs:
##########
@@ -336,6 +383,33 @@ impl RESTEnv {
}
}
+fn response_identifier(
+ requested: &Identifier,
+ response: &crate::api::GetTableResponse,
+) -> Result<Identifier> {
+ let database = response
+ .database
+ .as_deref()
+ .unwrap_or_else(|| requested.database());
+ let name = response.name.as_deref().ok_or_else(|| Error::DataInvalid {
+ message: format!("Table response for database '{database}' missing
name"),
+ source: None,
+ })?;
+ let identifier = Identifier::new(database, name);
+ identifier.validate()?;
+ if &identifier != requested {
Review Comment:
Fixed in apache/paimon#10123 commit `372b2e5ef7`: PyPaimon now passes
`get_object_name()`, and its REST test server returns the requested
branch-qualified name while still looking up base-table metadata. Added the
response-contract assertion and ran the real #922 binding branch cases: 2
passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
