Skip to content

Fix imdb_movie_picker: correct indentation, parameterize SQL, add err… - #123

Open
JoexBlaze89 wants to merge 1 commit into
DedSecInside:masterfrom
JoexBlaze89:master
Open

JoexBlaze89 wants to merge 1 commit into
DedSecInside:masterfrom
JoexBlaze89:master

Conversation

@JoexBlaze89

Copy link
Copy Markdown

…or handling, env-configurable API key and DB path

…or handling, env-configurable API key and DB path

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c6f77451f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Misc/imdb_movie_picker.py
continue

try:
resp = requests.get(f"http://www.omdbapi.com/?i={imdb_id}&apikey={API_KEY}", timeout=5)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Send configured API keys over HTTPS

When OMDB_API_KEY is configured, this request places that caller-provided credential in a plaintext http:// URL. On an untrusted network, an observer can recover the key and consume its OMDb quota; use the HTTPS endpoint for this request.

Useful? React with 👍 / 👎.

Comment thread Misc/imdb_movie_picker.py
Comment on lines +50 to +52
except Exception:
# network error or invalid json; try again
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Surface permanent request failures instead of retrying forever

If the configured key is invalid, OMDb returns a persistent HTTP error, or the network is unavailable, raise_for_status()/JSON parsing enters this handler and the outer while True immediately retries forever. The script consequently never tells the user why it cannot obtain a movie and can generate unbounded failed requests; bound retries or report unrecoverable errors.

Useful? React with 👍 / 👎.

Comment thread Misc/imdb_movie_picker.py
Comment on lines +72 to +75
CREATE TABLE IF NOT EXISTS movies_watched (
id INTEGER PRIMARY KEY AUTOINCREMENT,
imdb_id TEXT UNIQUE
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Migrate the legacy schema before relying on uniqueness

For a movie.db created by the previous version, movies_watched already exists with imdb_id INTEGER and no unique constraint. CREATE TABLE IF NOT EXISTS does not alter that schema, so INSERT OR IGNORE still inserts duplicate IMDb IDs despite the new function contract; migrate the existing table or create a unique index before relying on the constraint.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant