Skip to content
This repository has been archived by the owner on Nov 21, 2024. It is now read-only.

Multithreading Auth #842

Merged
merged 8 commits into from
Oct 28, 2024
Merged

Multithreading Auth #842

merged 8 commits into from
Oct 28, 2024

Conversation

PrzeG
Copy link
Collaborator

@PrzeG PrzeG commented Oct 21, 2024

Pull Request summary:

Adds support for multithreading authentication, which allows for concurrent requests using the same auth.

Usage example:

from threading import Thread
from catalystwan.session import ManagerSession
from catalystwan.vmanage_auth import vManageAuth
from copy import copy

def print_devices(manager: ManagerSession):
    # using context manager (recommended)
    with manager.login() as session:
        print(session.api.devices.get())

if __name__ =="__main__":

    # 1. Create shared authentication handler for user session
    auth = vManageAuth(username="username", password="password")
    # 2. Configure session with base url and attach authentication handler
    manager = ManagerSession(base_url="https://url:port", auth=auth)
    # 3. Make sure each thread gets own copy of ManagerSession object
    t1 = Thread(target=print_devices, args=(manager,))
    t2 = Thread(target=print_devices, args=(copy(manager),))
    t3 = Thread(target=print_devices, args=(copy(manager),))

    t1.start()
    t2.start()
    t3.start()

    t1.join()
    t2.join()
    t3.join()

    print("Done!")

Checklist:

  • Make sure to run pre-commit before committing changes
  • Make sure all checks have passed
  • PR description is clear and comprehensive
  • Mentioned the issue that this PR solves (if applicable)
  • Make sure you test the changes

@nikhilkp93 nikhilkp93 self-requested a review October 21, 2024 17:38
Comment on lines 435 to 439
if self._limiter is None:
response = super(ManagerSession, self).request(method, full_url, *args, **_kwargs)
else:
delayed_request = lambda: super(ManagerSession, self).request(method, full_url, *args, **_kwargs)
response = self._limiter.send_request(delayed_request)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can create dummy limiter and do it in this way:

Suggested change
if self._limiter is None:
response = super(ManagerSession, self).request(method, full_url, *args, **_kwargs)
else:
delayed_request = lambda: super(ManagerSession, self).request(method, full_url, *args, **_kwargs)
response = self._limiter.send_request(delayed_request)
with self._limiter:
response = super(ManagerSession, self).request(method, full_url, *args, **_kwargs)

def clear_sync(self, last_request: Optional[PreparedRequest]) -> None:
...

def register_session(self) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name suggest something else than it does.

Comment on lines 70 to 73
def clear(self) -> None:
...

def clear_sync(self, last_request: Optional[PreparedRequest]) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this moment method clear is not public api method. Maybe you can replace clear(...) by _clear(...) and clear_sync(...) by clear(...)

self.cookies.clear_session_cookies()
self._auth.clear()
self._auth.clear_sync(self._last_request)
self.auth = self._auth
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this line. Why we want make auth public after login. Do we need later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we need it. Auth is called with every request and requests library expects it in that place.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need it for requests :/

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
sbasan
sbasan previously approved these changes Oct 25, 2024
Copy link
Contributor

@sbasan sbasan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

JimOverholt
JimOverholt previously approved these changes Oct 27, 2024
@PrzeG PrzeG dismissed stale reviews from JimOverholt and sbasan via a993f6c October 28, 2024 09:59
@PrzeG PrzeG merged commit a668f58 into main Oct 28, 2024
11 checks passed
@PrzeG PrzeG deleted the multithreading_auth branch October 28, 2024 12:18
sbasan pushed a commit that referenced this pull request Oct 29, 2024
sbasan added a commit that referenced this pull request Oct 29, 2024
* csr generation as task for >=20.16 (#838)

* Bump actions/checkout from 4.2.0 to 4.2.1

Bumps [actions/checkout](https://github.com/actions/checkout) from 4.2.0 to 4.2.1.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@d632683...eef6144)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Multithreading Auth (#842)

* Bump actions/checkout from 4.2.1 to 4.2.2 (#846)

Bumps [actions/checkout](https://github.com/actions/checkout) from 4.2.1 to 4.2.2.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@eef6144...11bd719)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump abatilo/actions-poetry from 3.0.0 to 3.0.1 (#845)

Bumps [abatilo/actions-poetry](https://github.com/abatilo/actions-poetry) from 3.0.0 to 3.0.1.
- [Release notes](https://github.com/abatilo/actions-poetry/releases)
- [Changelog](https://github.com/abatilo/actions-poetry/blob/master/.releaserc)
- [Commits](abatilo/actions-poetry@7b6d33e...e78f54a)

---
updated-dependencies:
- dependency-name: abatilo/actions-poetry
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump actions/setup-python from 5.2.0 to 5.3.0 (#844)

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 5.2.0 to 5.3.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@f677139...0b93645)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* bump dev version

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: PrzeG <86780353+PrzeG@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants