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

K-allagbe/issue35-packaging-membrane #80

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

K-allagbe/issue35-packaging-membrane #80

wants to merge 24 commits into from

Conversation

k-allagbe
Copy link
Member

@k-allagbe k-allagbe commented Oct 6, 2023 •
edited
Loading

Closes #35
Fixes #85

issue35-packaging-membrane

The package is available in membrane.client.flask.
Usage description in the README.

Will add a jwt module for shared jwt management code with membrane-backend in #60.

Incoming refactor for automatic client registering in #79

@k-allagbe k-allagbe self-assigned this Oct 6, 2023
@k-allagbe k-allagbe requested a review from rngadam October 6, 2023 22:06
@k-allagbe k-allagbe force-pushed the k-allagbe/issue35-packaging-membrane branch from e54047c to d64b933 Compare October 6, 2023 23:11
rngadam
rngadam suggested changes Oct 7, 2023
View reviewed changes
Copy link
Contributor

@rngadam rngadam left a comment

Choose a reason for hiding this comment

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

also take care of linting fails.

@k-allagbe k-allagbe force-pushed the k-allagbe/issue35-packaging-membrane branch 2 times, most recently from 717bdd4 to 225c9f0 Compare October 10, 2023 05:43
@k-allagbe k-allagbe force-pushed the k-allagbe/issue35-packaging-membrane branch from 225c9f0 to 87906b5 Compare October 10, 2023 05:49
rngadam
rngadam suggested changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@rngadam rngadam left a comment

Choose a reason for hiding this comment

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

I believe there are at least two bugs that are going to surface once you do integration testing.

@k-allagbe k-allagbe marked this pull request as draft October 17, 2023 08:26
rngadam
rngadam suggested changes Oct 19, 2023
View reviewed changes
@k-allagbe k-allagbe requested review from vivalareda and removed request for vivalareda October 19, 2023 18:29
rngadam
rngadam suggested changes Oct 20, 2023
View reviewed changes
Copy link
Contributor

@rngadam rngadam left a comment

Choose a reason for hiding this comment

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

Your vscode configuration references pytest but you actually use unittest

image

rngadam
rngadam approved these changes Oct 23, 2023
View reviewed changes
Copy link
Contributor

@rngadam rngadam left a comment

Choose a reason for hiding this comment

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

according to our conversation, you were able to successfully test this with two deployed apps so for me we can move to new issues (recording any remaining work as new issues).

@k-allagbe k-allagbe requested a review from rngadam October 23, 2023 21:03
@k-allagbe k-allagbe marked this pull request as ready for review October 23, 2023 21:03
rngadam
rngadam suggested changes Oct 23, 2023
View reviewed changes
@k-allagbe k-allagbe requested a review from rngadam October 23, 2023 21:47
rngadam
rngadam suggested changes Oct 30, 2023
View reviewed changes
config.py

@dataclass
class JWTConfig:
""""""
Copy link
Contributor

Choose a reason for hiding this comment

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

many one-line doc string missing across code base

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rngadam rngadam rngadam requested changes

At least 1 approving review is required to merge this pull request.

Assignees

@k-allagbe k-allagbe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor unit tests for membrane-backend Solution to make the project an importable library
2 participants
@k-allagbe @rngadam