Fix #747 by updating async SQLAlchemy OAuth example code #748
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Neel Arora seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
ntarora
commented
Oct 23, 2022
|
fix #747 |
seratch
added
docs
seratch
changed the title
seratch
requested changes
Oct 24, 2022
| @@ -63,11 +63,11 @@ async def async_save(self, installation: Installation): | |||
| b["client_id"] = self.client_id | |||
| await database.execute(self.bots.insert(), b) | |||
|
|
|||
| async def async_find_bot(self, *, enterprise_id: Optional[str], team_id: Optional[str]) -> Optional[Bot]: | |||
| async def async_find_bot(self, *, enterprise_id: Optional[str], team_id: Optional[str], is_enterprise_install: Optional[bool]) -> Optional[Bot]: | |||
There was a problem hiding this comment.
flake8 detects lint errors. Can you these lines shorter than 125 chars?
Successfully installed flake8-5.0.4 mccabe-0.7.0 pycodestyle-2.9.1 pyflakes-2.5.0
examples/sqlalchemy/async_oauth_app.py:66:126: E501 line too long (148 > 125 characters)
examples/sqlalchemy/async_oauth_app.py:70:126: E501 line too long (130 > 125 characters)
Error: Process completed with exit code 1.
There was a problem hiding this comment.
You can fix these warnings by the following changes:
diff --git a/examples/sqlalchemy/async_oauth_app.py b/examples/sqlalchemy/async_oauth_app.py
index 5ec7be0..a2c52f0 100644
--- a/examples/sqlalchemy/async_oauth_app.py
+++ b/examples/sqlalchemy/async_oauth_app.py
@@ -63,11 +63,23 @@ class AsyncSQLAlchemyInstallationStore(AsyncInstallationStore):
b["client_id"] = self.client_id
await database.execute(self.bots.insert(), b)
- async def async_find_bot(self, *, enterprise_id: Optional[str], team_id: Optional[str], is_enterprise_install: Optional[bool]) -> Optional[Bot]:
+ async def async_find_bot(
+ self,
+ *,
+ enterprise_id: Optional[str],
+ team_id: Optional[str],
+ is_enterprise_install: Optional[bool],
+ ) -> Optional[Bot]:
c = self.bots.c
query = (
self.bots.select()
- .where(and_(c.enterprise_id == enterprise_id, c.team_id == team_id, c.is_enterprise_install == is_enterprise_install))
+ .where(
+ and_(
+ c.enterprise_id == enterprise_id,
+ c.team_id == team_id,
+ c.is_enterprise_install == is_enterprise_install,
+ )
+ )
.order_by(desc(c.installed_at))
.limit(1)
)There was a problem hiding this comment.
@ntarora Could you push the fix if you have a chance?
Codecov Report
@@ Coverage Diff @@
## main #748 +/- ##
=======================================
Coverage 92.06% 92.06%
=======================================
Files 172 172
Lines 5875 5875
=======================================
Hits 5409 5409
Misses 466 466 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Odd, I did push the formatting fix on my branch. I didn’t get picked up for some reason. Should I close this PR and create a new one? On Oct 26, 2022, at 1:39 AM, Kazuhiro Sera ***@***.***> wrote:
@seratch commented on this pull request.
In examples/sqlalchemy/async_oauth_app.py:
@@ -63,11 +63,11 @@ async def async_save(self, installation: Installation):
b["client_id"] = self.client_id
await database.execute(self.bots.insert(), b)
- async def async_find_bot(self, *, enterprise_id: Optional[str], team_id: Optional[str]) -> Optional[Bot]:
+ async def async_find_bot(self, *, enterprise_id: Optional[str], team_id: Optional[str], is_enterprise_install: Optional[bool]) -> Optional[Bot]:
@ntarora Could you push the fix if you have a chance?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
seratch
pushed a commit
that referenced
this pull request
Nov 4, 2022
|
Thanks for your contribution! Your changes are merged at f5b40c7 and then I've added an additional commit to resolve the flake8 issue on my end. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request resolves #747