Skip to content

Fixes #5 #6

Merged
gbyx3 merged 5 commits into
axians:developmentfrom
Utbildadninja:Fix_Issue_5
Nov 1, 2024
Merged

Fixes #5 #6
gbyx3 merged 5 commits into
axians:developmentfrom
Utbildadninja:Fix_Issue_5

Conversation

@Utbildadninja

Copy link
Copy Markdown
Contributor

Fixes #5 by converting the expected string to an integer.
Reset to default TTL when encountering reasonable exceptions.
Log exception and reset to default TTL when encountering unexpected exceptions.

@gbyx3 gbyx3 linked an issue Oct 11, 2024 that may be closed by this pull request
@gbyx3

gbyx3 commented Oct 11, 2024

Copy link
Copy Markdown
Member

I think it would be better to just skip the isinstance int on L283 and cast the ttl value as an integer

Reverted number to text in index to avoid adding arrows to input field.
Convert TTL to integer or throw an error.
Combined nested if statements
@Utbildadninja

Copy link
Copy Markdown
Contributor Author

I think it would be better to just skip the isinstance int on L283 and cast the ttl value as an integer

Agreed, keeping that change. TTL is now cast to int, error forwarded to user if something goes wrong. Combined nested if statements as suggested.

I was thinking, should we handle negative ttl values as well?

@gbyx3

gbyx3 commented Oct 23, 2024

Copy link
Copy Markdown
Member

I was thinking, should we handle negative ttl values as well?

You could try something like:

try:
    ttl = int(request_body["ttl"])
    if ttl <= 0:
        raise ValueError("The number must be a positive integer.")
except ValueError as e:
    print(e)

Handle negative TTL
@gbyx3

gbyx3 commented Oct 31, 2024

Copy link
Copy Markdown
Member

Looks good, we try for a Friday merge! 🙏

@gbyx3 gbyx3 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@gbyx3 gbyx3 merged commit 0967d90 into axians:development Nov 1, 2024
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.

[BUG] TTL needs to be sent as a integer.

2 participants