turbaszek commented on pull request #9563: URL: https://github.com/apache/airflow/pull/9563#issuecomment-651065623
> What happens if we did `session.add(self)` instead? We already have an TI object that should be updated, so we could likely just let SQLA handle this all for us. As Kaxil mentioned: `Original exception was: (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "task_instance_pkey"` even if there's no ti in session. > In terms of performance: I don't think we should aim to reduce the number of queries above all else - our primary goal should be clarify of code/intent, and only for critical/hot paths should we then go for performance above readable code, and as this is so far off the critical path (as it happens on the worker) that this won't have much if any impact on performance of Airflow as a whole. And even for a single task this only adds micro-seconds (one DB round trip) The readability can be improved by extracting such logic to custom method/function (as we do in many places). I agree that at first look this is not a breakthrough in performance but on the other hand it is an enhancement. > At the very least if we go with this approach we should have a clear comment saying _why_ we are using this approach. I agree. ---------------------------------------------------------------- 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: [email protected]
