blackmwk commented on PR #3090: URL: https://github.com/apache/iceberg-rust/pull/3090#issuecomment-5489582412
hi, @laskoviymishka thanks for review, I addressed all comments, PTAL. > What's left is that the loader now hard-errors on serialize while the secret strings — s3.secret-access-key, s3.session-token, GCS service-account JSON, hf.token — still pass through in plaintext (and test_memory_file_io_serialization_roundtrip pins that). The asymmetry reads backwards: we fail fast on the opaque loader handle and silently emit the values most dangerous to leak. I'd really like by-default redaction of the well-known credential keys before we merge, with full-fidelity passthrough as an explicit opt-in. While I agree that we should redact sensitive credentials, I don't think maintaining a predefine set of well know keys is the right direction. I think https://github.com/apache/iceberg-rust/issues/3129 is the right direction to go, e.g. replacing raw hash map with a config struct. For now I've removed the derived easy to use ser/de traits, and ask user to use explict `serialize_all` methods to call it. WYDT? -- 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]
