-
-
Notifications
You must be signed in to change notification settings - Fork 218
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
Add option to not Input data during login and raise TwitterException #284
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new parameter, Sequence diagram for login flow with input_for_login parametersequenceDiagram
participant User as User
participant Client as Client
participant Twitter as Twitter API
User->>Client: login(auth_info, password, input_for_login)
Client->>Twitter: Initiate login flow
Twitter-->>Client: Account locked response
alt input_for_login = True
Client->>User: Show prompt for verification
User->>Client: Enter verification text
Client->>Twitter: Submit verification
Twitter-->>Client: Login response
Client-->>User: Return login response
else input_for_login = False
Client-->>User: Raise TwitterException
end
Class diagram showing updated Client class with new parameterclassDiagram
class Client {
+login(auth_info_1: str, auth_info_2: str|None, password: str, totp_secret: str|None, input_for_login: bool) dict
}
note for Client "input_for_login parameter added
defaults to True for backward compatibility"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant Client
participant LoginSystem
participant User
alt input_for_login is True
Client->>LoginSystem: Attempt Login
LoginSystem-->>Client: Account Locked
Client->>User: Prompt for Input
User->>Client: Provide Input
else input_for_login is False
Client->>LoginSystem: Attempt Login
LoginSystem-->>Client: Account Locked
Client->>Client: Raise TwitterException
end
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Hey @luishacm - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add test cases to verify behavior for both input_for_login=True and input_for_login=False scenarios
- Consider updating the documentation to explicitly state that when input_for_login is False, a TwitterException will be raised if the account is locked
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
twikit/client/client.py (1)
431-442
: Consider these minor improvements.
- Remove the empty line at line 441 for better code consistency.
- Add a return type hint to the login method signature to improve type safety:
async def login( self, *, auth_info_1: str, auth_info_2: str | None = None, password: str, totp_secret: str | None = None, input_for_login: bool = True - ) -> dict: + ) -> dict[str, Any]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
twikit/client/client.py
(3 hunks)
🔇 Additional comments (2)
twikit/client/client.py (2)
290-291
: LGTM! Well-documented parameter addition.The new
input_for_login
parameter is clearly named and well-documented. The default value ofTrue
maintains backward compatibility for existing users.Also applies to: 315-316
431-442
: LGTM! Clean implementation of the login control flow.The conditional logic properly handles both interactive and non-interactive scenarios, maintaining existing behavior when
input_for_login
isTrue
and raising an appropriate exception whenFalse
.
This solves automatic login flows problems. |
Changes
This PR introduces the
input_for_login
parameter to theClient
class method, enhancing the flexibility of the login process. The newly addedinput_for_login
parameter allows users to control whether an input prompt is shown when the account is locked.What's Changed
input_for_login
parameterinput_for_login
, with a default value ofTrue
, was added to the login method signature.Updated Docstring
input_for_login
parameter.Testing
Minimal changes have been made to the login flow, and existing tests should cover the updated method.
Additional manual verification was performed to ensure that
input_for_login
behaves as expected.Notes
No breaking changes were introduced.
Maintains backward compatibility by setting the default value of
input_for_login
toTrue
.Summary by Sourcery
Add an
input_for_login
parameter to theClient.login
method to control whether to prompt for input when the account is locked.New Features:
Tests:
Summary by CodeRabbit
New Features
Documentation