-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update sample apps for Python SDK v5 #59
Conversation
@@ -1,14 +1,14 @@ | |||
certifi==2021.5.30 | |||
charset-normalizer==2.0.6 | |||
click==8.0.1 | |||
Flask==2.0.1 | |||
Flask==2.0.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different examples used different flask versions. I set them all to the same version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason we can't bump to the latest Flask version? Also fine with keeping this out of scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason I didn't bump up to latest version was to limit the scope of changes as I swept through and updated the sample apps. Basically, time savings for me in case bumping up to latest Flask triggered other code or dependency changes.
flask-lucide==0.2.0 | ||
Werkzeug==2.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting flask would error without this dependency
@@ -1,14 +1,14 @@ | |||
certifi==2021.5.30 | |||
charset-normalizer==2.0.6 | |||
click==8.0.1 | |||
Flask==2.0.1 | |||
Flask==2.0.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason we can't bump to the latest Flask version? Also fine with keeping this out of scope.
workos.api_key = os.getenv("WORKOS_API_KEY") | ||
workos.project_id = os.getenv("WORKOS_CLIENT_ID") | ||
workos.base_api_url = "http://localhost:7000/" if DEBUG else workos.base_api_url | ||
base_api_url = "http://localhost:7000/" if DEBUG else None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels kind of odd to me for a public example. I wonder if we should just use the WORKOS_BASE_URL
environment variable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. This is a simple change I can make to all of the sample apps.
Update all of the flask examples to work with v5 of the SDK.
I touched up the Directory Sync app a bit to add more paths that exercise more SDK paths. For the other example apps, I just made enough changes to keep them functional and not introduce type errors when passing arguments to the SDK.