Vamsi-klu commented on PR #69689: URL: https://github.com/apache/airflow/pull/69689#issuecomment-4966077210
Reads correct to me. On the classic msrest models `response.run_id` and `response.id` resolve to the same values `vars(response)[...]` was pulling out of `__dict__`, and the plain attribute access is what keeps the v10 hybrid models working since those expose the fields through property descriptors. Worth noting the `run_spark_job` hook already sets `self.job_id = job.id` internally, so the synapse `execute()` change just lines up with what the hook was doing. The `HybridModelResponse` regression tests that only expose the value via a property are the right shape here, since they'd fail on the old `vars()` path. Only nit, this trades a possible `KeyError` for an `AttributeError` if the SDK ever drops the field, but the old code already assumed the key was present so it's no worse than before. -- 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]
