Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 146: Autenticated client was overriding the manager and it was causing issue #149

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

olethanh
Copy link
Contributor

…using issue

Psycojoker
Psycojoker previously approved these changes Aug 14, 2024
@olethanh olethanh changed the title Fix 146: Ah client was overrinding the manager Fix 146: Autenticated client was overriding the manager and it was causing issue Aug 14, 2024
@Psycojoker Psycojoker merged commit 0de453d into main Aug 14, 2024
14 checks passed
@Psycojoker Psycojoker deleted the lo-fix-146 branch August 14, 2024 15:33
@github-actions github-actions bot added the BLACK This PR has critical implications and must be reviewed by a senior engineer. label Aug 14, 2024
Copy link


Summary:
This PR introduces a refactoring of the asynchronous context management methods in the AuthenticatedAlephHttpClient and AlephHttpClient classes. The changes include:

  1. Removing the __aenter__ method from AuthenticatedAlephHttpClient.
  2. Modifying the __aexit__ method in AlephHttpClient to ensure the HTTP session is closed only if it has been initialized.

Explanation:
The removal of the __aenter__ method from AuthenticatedAlephHttpClient simplifies the class and removes unnecessary functionality. The modification in AlephHttpClient ensures that the HTTP session is properly closed even in case of an error, preventing potential resource leaks.

These changes are significant as they affect the handling of asynchronous operations and resource management, which could potentially impact the stability and performance of the application. Therefore, a thorough review is advised to ensure compatibility with existing workflows and to minimize the risk of introducing bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLACK This PR has critical implications and must be reviewed by a senior engineer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants