korbit-ai[bot] commented on code in PR #33300: URL: https://github.com/apache/superset/pull/33300#discussion_r2068893284
########## superset/utils/metadata.py: ########## @@ -0,0 +1,9 @@ +from superset import db + +class SuspendSession: + def __enter__(self): + self.session_objects = db.session.identity_map.values() + db.session.remove() + def __exit__(self, exc_type, exc_val, exc_tb): + for obj in self.session_objects: + db.session.add(obj) Review Comment: ### Inefficient Sequential Session Object Reattachment <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Adding objects back to the session one at a time in a loop is inefficient for large numbers of objects. ###### Why this matters For sessions with many objects, the individual add operations create unnecessary overhead that could impact performance when reattaching objects to the session. ###### Suggested change ∙ *Feature Preview* Use SQLAlchemy's bulk operations to reattach all objects at once: ```python def __exit__(self, exc_type, exc_val, exc_tb): if self.session_objects: db.session.bulk_save_objects(self.session_objects) ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e612c774-c7a5-48c2-8561-61b9cef0c6c2/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e612c774-c7a5-48c2-8561-61b9cef0c6c2?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e612c774-c7a5-48c2-8561-61b9cef0c6c2?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e612c774-c7a5-48c2-8561-61b9cef0c6c2?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e612c774-c7a5-48c2-8561-61b9cef0c6c2) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:ed503862-ccf2-483a-a7b8-a5d683592b8f --> [](ed503862-ccf2-483a-a7b8-a5d683592b8f) ########## superset/utils/metadata.py: ########## @@ -0,0 +1,9 @@ +from superset import db + +class SuspendSession: + def __enter__(self): + self.session_objects = db.session.identity_map.values() + db.session.remove() + def __exit__(self, exc_type, exc_val, exc_tb): + for obj in self.session_objects: + db.session.add(obj) Review Comment: ### Incomplete Session Management <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The context manager doesn't explicitly recreate a new session after removal, which could lead to undefined behavior. ###### Why this matters Without explicitly creating a new session after removal, subsequent database operations might fail or interact with an invalid session state. ###### Suggested change ∙ *Feature Preview* Add explicit session creation in __exit__: ```python def __exit__(self, exc_type, exc_val, exc_tb): db.session.begin() for obj in self.session_objects: db.session.add(obj) ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a91bcc6a-8a03-41f8-8cd0-c2a437e59363/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a91bcc6a-8a03-41f8-8cd0-c2a437e59363?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a91bcc6a-8a03-41f8-8cd0-c2a437e59363?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a91bcc6a-8a03-41f8-8cd0-c2a437e59363?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a91bcc6a-8a03-41f8-8cd0-c2a437e59363) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:dd833d3e-98c4-4a32-ae05-a3b10c6ba596 --> [](dd833d3e-98c4-4a32-ae05-a3b10c6ba596) -- 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]
