potiuk commented on PR #63663:
URL: https://github.com/apache/airflow/pull/63663#issuecomment-4065526454

   @Gautam-Bharadwaj This PR has a few issues that need to be addressed before 
it can be reviewed — please see our [Pull Request quality 
criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   
   **Issues found:**
   - :warning: **Testing Requirements**: The PR changes behavior in 
`merge_conn` and `add_default_pool_if_not_exists` by replacing 
`session.commit()` with `session.flush()`. This is a potentially breaking 
change for callers that relied on the commit being issued inside these 
functions. No new or updated tests are included to demonstrate that the 
behavior is correct under the new semantics (e.g., that callers still see 
persisted data after calling these utilities). A regression test that exercises 
the `@provide_session`-managed path without an outer transaction, as well as 
the caller-managed-session path, would be appropriate.
   
   > **Note:** Your branch is **21 commits behind `main`**. Some check failures 
may be caused by changes in the base branch rather than by your PR. Please 
rebase your branch and push again to get up-to-date CI results.
   
   **What to do next:**
   - The comment informs you what you need to do.
   - Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - 
but only after making sure that all the issues are fixed.
   - There is no rush — take your time and work at your own pace. We appreciate 
your contribution and are happy to wait for updates.
   - Maintainers will then proceed with a normal review.
   
   There is no rush — take your time and work at your own pace. We appreciate 
your contribution and are happy to wait for updates. If you have questions, 
feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).


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

Reply via email to