Skip to content

fix: Explicit NEEDS_APPLICATION_ID declaration #87

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.

Sign up for GitHub

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

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

fix: Explicit NEEDS_APPLICATION_ID declaration #87

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed36f3d
fix: Explicit NEEDS_APPLICATION_ID declaration
GuyKh Aug 28, 2025
e34d065
Update __init__.py
GuyKh Aug 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions trakt/auth/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# -*- coding: utf-8 -*-
"""Authentication methods"""

__author__ = 'Jon Nappi, Elan Ruusamäe'
__author__ = "Jon Nappi, Elan Ruusamäe"

from getpass import getpass

from trakt.config import AuthConfig
from trakt.core import DEVICE_AUTH, OAUTH_AUTH, PIN_AUTH, api
Expand All @@ -20,32 +22,42 @@ def oauth_auth(*args, config, **kwargs):
return OAuthAdapter(*args, client=api(), config=config, **kwargs).authenticate()


def device_auth(config):
def device_auth(*args, config, **kwargs):
from trakt.auth.device import DeviceAuthAdapter

return DeviceAuthAdapter(client=api(), config=config).authenticate()


# Expose NEEDS_APPLICATION_ID on wrapper functions so init_auth can detect it
pin_auth.NEEDS_APPLICATION_ID = True
oauth_auth.NEEDS_APPLICATION_ID = False
device_auth.NEEDS_APPLICATION_ID = False


def get_client_info(app_id: bool, config: AuthConfig):
"""Helper function to poll the user for Client ID and Client Secret
strings

:return: A 2-tuple of client_id, client_secret
"""
print('If you do not have a client ID and secret. Please visit the '
'following url to create them.')
print('https://trakt.tv/oauth/applications')
client_id = input('Please enter your client id: ')
client_secret = input('Please enter your client secret: ')
print(
"If you do not have a client ID and secret. Please visit the "
"following url to create them."
)
print("https://trakt.tv/oauth/applications")
client_id = input("Please enter your client id: ")
client_secret = getpass("Please enter your client secret: ")
if app_id:
msg = f'Please enter your application ID ({config.APPLICATION_ID}): '
msg = f"Please enter your application ID ({config.APPLICATION_ID}): "
user_input = input(msg)
if user_input:
config.APPLICATION_ID = user_input
return client_id, client_secret
Comment on lines +43 to 55
Copy link

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make prompts resilient and reuse existing config values.
Allow pressing Enter to keep preloaded values; don’t force retyping secrets.

-    print(
-        "If you do not have a client ID and secret. Please visit the "
-        "following url to create them."
-    )
-    print("https://trakt.tv/oauth/applications")
-    client_id = input("Please enter your client id: ")
-    client_secret = getpass("Please enter your client secret: ")
+    print(
+        "If you do not have a client ID and secret. Please visit the "
+        "following url to create them."
+    )
+    print("https://trakt.tv/oauth/applications")
+    default_client_id = config.CLIENT_ID or ""
+    cid_prompt = "Please enter your client id"
+    if default_client_id:
+        cid_prompt += f" ({default_client_id})"
+    cid_prompt += ": "
+    client_id = input(cid_prompt) or default_client_id
+
+    secret_prompt = "Please enter your client secret (leave blank to keep existing): "
+    secret_input = getpass(secret_prompt)
+    client_secret = secret_input or (config.CLIENT_SECRET or "")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
print(
"If you do not have a client ID and secret. Please visit the "
"following url to create them."
)
print("https://trakt.tv/oauth/applications")
client_id = input("Please enter your client id: ")
client_secret = getpass("Please enter your client secret: ")
if app_id:
msg = f'Please enter your application ID ({config.APPLICATION_ID}): '
msg = f"Please enter your application ID ({config.APPLICATION_ID}): "
user_input = input(msg)
if user_input:
config.APPLICATION_ID = user_input
return client_id, client_secret
print(
"If you do not have a client ID and secret. Please visit the "
"following url to create them."
)
print("https://trakt.tv/oauth/applications")
# Allow keeping existing client ID
default_client_id = config.CLIENT_ID or ""
cid_prompt = "Please enter your client id"
if default_client_id:
cid_prompt += f" ({default_client_id})"
cid_prompt += ": "
client_id = input(cid_prompt) or default_client_id
# Allow keeping existing client secret
secret_prompt = "Please enter your client secret (leave blank to keep existing): "
secret_input = getpass(secret_prompt)
client_secret = secret_input or (config.CLIENT_SECRET or "")
if app_id:
msg = f"Please enter your application ID ({config.APPLICATION_ID}): "
user_input = input(msg)
if user_input:
config.APPLICATION_ID = user_input
return client_id, client_secret
🤖 Prompt for AI Agents 
In trakt/auth/__init__.py around lines 43 to 55, the interactive prompts always
force the user to retype values and expose no defaults; update prompts to
display existing config values as defaults and accept empty input (Enter) to
keep them: if config.CLIENT_ID or config.CLIENT_SECRET exist show them in the
prompt and when input is empty preserve the existing value (for secrets use
getpass but treat blank as “keep existing” without printing), and for
APPLICATION_ID keep the current config.APPLICATION_ID as the default and only
replace it if the user types a non-empty value; finally return the possibly
unchanged client_id and client_secret.



def init_auth(method: str, *args, client_id=None, client_secret=None, store=False, **kwargs):
def init_auth(
method: str, *args, client_id=None, client_secret=None, store=False, **kwargs
):
Comment on lines +58 to +60
Copy link

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: store parameter is unused (functional regression).
Docstring advertises persistence via store, but there is no call to config.store(). Either implement persistence or remove the param/docs.

See proposed fix in Lines 86.

🤖 Prompt for AI Agents 
In trakt/auth/__init__.py around lines 58-60, the init_auth function accepts a
store parameter but never persists the resulting config, causing a functional
regression; modify the function so that after the config is created/updated
(where config variable is available, e.g., after login/creation at the later
lines around 86), if store is True call config.store() (handle/propagate any
exceptions or log errors as appropriate) so the configuration is actually
persisted; alternatively, if persistence is intentionally removed, remove the
store parameter and update the docstring accordingly.

"""Run the auth function specified by *AUTH_METHOD*

:param store: Boolean flag used to determine if your trakt api auth data
Expand All @@ -60,16 +72,15 @@ def init_auth(method: str, *args, client_id=None, client_secret=None, store=Fals
}

config = config_factory()
adapter = methods.get(method, PIN_AUTH)
adapter = methods.get(method, pin_auth)

"""
Update client_id, client_secret from input or ask them interactively
"""
if client_id is None and client_secret is None:
client_id, client_secret = get_client_info(adapter.NEEDS_APPLICATION_ID, config)
client_id, client_secret = get_client_info(
getattr(adapter, "NEEDS_APPLICATION_ID", False), config
)
config.CLIENT_ID, config.CLIENT_SECRET = client_id, client_secret
Comment on lines 80 to 84
Copy link

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Prompt when either client_id or client_secret is missing; keep provided values intact.
Currently prompts only when both are None; handle partial input.

-    if client_id is None and client_secret is None:
-        client_id, client_secret = get_client_info(
-            getattr(adapter, "NEEDS_APPLICATION_ID", False), config
-        )
+    if client_id is None or client_secret is None:
+        entered_id, entered_secret = get_client_info(
+            getattr(adapter, "NEEDS_APPLICATION_ID", False), config
+        )
+        client_id = client_id or entered_id
+        client_secret = client_secret or entered_secret
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if client_id is None and client_secret is None:
client_id, client_secret = get_client_info(adapter.NEEDS_APPLICATION_ID, config)
client_id, client_secret = get_client_info(
getattr(adapter, "NEEDS_APPLICATION_ID", False), config
)
config.CLIENT_ID, config.CLIENT_SECRET = client_id, client_secret
if client_id is None or client_secret is None:
entered_id, entered_secret = get_client_info(
getattr(adapter, "NEEDS_APPLICATION_ID", False), config
)
client_id = client_id or entered_id
client_secret = client_secret or entered_secret
config.CLIENT_ID, config.CLIENT_SECRET = client_id, client_secret
🤖 Prompt for AI Agents 
In trakt/auth/__init__.py around lines 80 to 84, the code only prompts for
credentials when both client_id and client_secret are None; change it to prompt
when either is missing and preserve any provided value. Specifically, if
client_id is None or client_secret is None call get_client_info(getattr(adapter,
"NEEDS_APPLICATION_ID", False), config), then only overwrite the missing values
with the returned tuple (i.e., keep any non-None input intact), and finally
assign config.CLIENT_ID and config.CLIENT_SECRET from the merged values.


adapter(*args, config=config, **kwargs)
Copy link

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Return adapter result and persist when requested.
Preserve return value semantics and implement store=True.

-    adapter(*args, config=config, **kwargs)
+    result = adapter(*args, config=config, **kwargs)
+    if store:
+        config.store()
+    return result
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
adapter(*args, config=config, **kwargs)
result = adapter(*args, config=config, **kwargs)
if store:
config.store()
return result
🤖 Prompt for AI Agents 
In trakt/auth/__init__.py around line 86, the adapter invocation currently
doesn't return its result nor handle persistence; change it to assign the
adapter call to a variable (result = adapter(*args, config=config, **kwargs)),
then if persistence is requested (e.g., config.get("store") is True or a store
flag in kwargs) call the configured persistence API to save the result (for
example invoke config.persist(result) or config["storage"].save(result)
depending on how config exposes storage), and finally return result so
return-value semantics are preserved.


if store:
config.store()