harshm-dev edited a comment on pull request #3326:
URL: https://github.com/apache/iceberg/pull/3326#issuecomment-948406775


   @kbendick Thanks for review! Really appreciate it. 
   
   > See how both type and catalog-type are set? Can we please consider another 
term? Additionally, this definitely needs to go into the Flink docs as well. 
And likely the spec as well given how large of a change it is. I'm not one to 
use Request changes, but could you please ensure this works with Flink or just 
change the term name from type? I think it's going to cause a lot of headaches, 
even if it seems like the most appropriate name. It's already taken 
essentially.🙁
   
   [harsh] 'type' is an existing configuration for spark/hive. So, not 
introducing a new conf - 
https://iceberg.apache.org/spark-configuration/#catalog-configuration , 
previously defined in the code 
[[here](https://github.com/apache/iceberg/blob/119d20d22e66655bce1590041e4ba25ae48874fb/core/src/main/java/org/apache/iceberg/CatalogUtil.java#L58)]
   This change is unlikely to impact Flink, because it's in a different code 
path. I'm planning to make a similar change in FlinkCatalogFactory in a 
different PR.


-- 
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to