jroachgolf84 commented on PR #51893: URL: https://github.com/apache/airflow/pull/51893#issuecomment-3000713998
> I don't love the idea of storing `self.merged_extra` and would like to see if we can cleanly avoid doing it > > If I'm following the logic right (and there's a decent chance I'm not) all the stuff we store in `self.merge_extra` is already set as a property on the session object which we return. > > If that is the case isn't session already configured and we don't need merged_extra again after this? @ashb - here is the explanation as to why this was implemented. It boils down to the `get_conn` method. In `run`, we call `get_conn`. This is where the session is created via the `_configure_session_from_extra`. However, there are additional fields NOT included in the session that will be used later, meaning that we need to hold onto some form of `extra_options`. Eventually, some form of `extra_options` needs to be passed to `run_and_check`, which is looking for the `timeout`, `allow_redirects`, and `check_response` fields. This mapping should be the merged mapping, which will include the `timeout`, `allow_redirects`, and `check_response` fields that could have been either in the Connection "extras" **or** the original `extra_options`. If we didn't create something like `self.merged_extra`, we'd need to change the return type of `get_conn`, which would be a breaking change. -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
