[GitHub] [airflow] dstandish commented on a change in pull request #15100: Add support for arbitrary json in conn uri format

[GitBox <gi...@apache.org>]

dstandish commented on a change in pull request #15100:
URL: https://github.com/apache/airflow/pull/15100#discussion_r606722251



##########
File path: tests/secrets/test_local_filesystem.py
##########
@@ -222,9 +222,9 @@ def test_missing_file(self, mock_exists):
                 {
                     "conn_a": "mysql://hosta",
                     "conn_b": ''.join(

Review comment:
       Not quite.
   
   I did not change the _connection_ at all.  Your yaml still parses to the same connection object.  But when parsed back to a URI, it actually became a different connection object; if you took that original uri and made it a connection, it would not have equalled the connection produced by your yaml, because it would have produced a different `extra`.
   
   After my change, this would no longer be the case, since it can now represent the connection produced by your yaml without loss.
   
   In my [most recent commit](https://github.com/apache/airflow/pull/15100/commits/7c6a02ae670ef5d87fedeb90e2f32598dbba0c5c#diff-ec0b1f1eed6ee36d376bf7340a10f5b7c6b57602ab56faf369b102482e9bd922L222-L229) I sidestep this issue entirely by comparing attributes instead of using get URI.  Now we test directly what your yaml conn parses to, without touching get_uri.
   
   




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

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