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

Refactor/reuse reqwest client #123

Merged
merged 7 commits into from
Jun 21, 2023
Merged

Conversation

bitfl0wer
Copy link
Member

This PR closes the issue described in #120.

@bitfl0wer bitfl0wer linked an issue Jun 20, 2023 that may be closed by this pull request
@SpecificProtagonist
Copy link
Contributor

Looks good^^ Of course there are still a lot (41) of other calls to Client::new that need to be removed, but the approach of one Client per Instance is sound.

@bitfl0wer
Copy link
Member Author

bitfl0wer commented Jun 21, 2023

there are still a lot (41) of other calls to Client::new that need to be removed

Do they, though? These Client::new() calls are just to make a RequestBuilder, which holds the information about the Request to be sent. This RequestBuilder then gets passed to the Instances' client, which executes the request and parses the information.

The connection pool of the one client which executes the Requests can still be held this way, I'd assume.

@bitfl0wer bitfl0wer merged commit da9ba97 into main Jun 21, 2023
@bitfl0wer bitfl0wer deleted the refactor/reuse-reqwest-client branch July 18, 2023 14:29
@bitfl0wer bitfl0wer mentioned this pull request Dec 14, 2023
bitfl0wer added a commit that referenced this pull request Dec 14, 2023
Title: Add User Authentication Feature

Description:

This pull request introduces user authentication functionality to our
web application. The main goal of this feature is to ensure that each
action performed on the platform is tied to a valid, logged-in user,
thus providing accountability and maintaining data security.

Changes:

New models: Added the User model to represent users in our system. This
model includes fields: username, password_hash, email etc.

User seriliazer and views: Implemented serializers and API views for
user registration, login, and logout.

Authentication Middlewares: Added middlewares to check for a valid
session or token before allowing access to certain views.

Tests: Included comprehensive test coverage for the new feature. Tests
were implemented to verify user registration, login, and logout
functionality, as well as checking authentication enforcement on
applicable views.

    Documentation: Updated API documentation related to User endpoints.

This feature is expected to improve the overall security of our
application by properly managing user sessions and actions.

Note: You will see references to some helper tools like make_password
and check_password methods, these are security measures to ensure we are
not storing plain text passwords in the database.

This PR follows our Python coding standards and is fully linted and
tested.

Requesting review and feedback. If all points are clear and no issues
are detected during the review, we would appreciate it if this PR could
be merged at the earliest convenience.

Related Issue: #123
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reuse reqwest client
2 participants