mgmarino commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1462802332


##########
tests/catalog/integration_test_glue.py:
##########
@@ -279,6 +379,20 @@ def test_commit_table_update_schema(
     assert test_catalog._parse_metadata_version(table.metadata_location) == 0
     assert original_table_metadata.current_schema_id == 0
 
+    assert athena.get_query_results(f'SELECT * FROM 
"{database_name}"."{table_name}"') == [

Review Comment:
   So, I did think about this, and my reasoning was as follows:
   
   - Generally, I don't think integration tests can be as granular as the unit 
tests.
   - when creating a table in Glue, I think it is reasonable to assert that 
that table should also be queryable with Athena. In other words, if the table 
was created and not queryable, then a table was not actually successfully 
created. 
   - similar point with the schema update, if a schema was updated and not 
queryable, then the schema was not successfully updated.
   
   As far as bug isolation, I would probably want rather unit tests to take the 
lead here.
   
   Anyways, this was just my reasoning, I'm happy to split out if you'd rather 
have it that way.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to