Skip to content

Add Conflict error (HTTP error code 409) - #1154

Merged
jsmnbom merged 4 commits into
masterfrom
fix-1143
Oct 29, 2018
Merged

Add Conflict error (HTTP error code 409)#1154
jsmnbom merged 4 commits into
masterfrom
fix-1143

Conversation

@jsmnbom

@jsmnbom jsmnbom commented Jul 8, 2018

Copy link
Copy Markdown
Member

Fixes #1143.

Do we need tests for stuff like this?

@Eldinnie

Copy link
Copy Markdown
Member

hmm, Not to be blunt but why should we patch a user_error? I think the case in #1143 is too specific to add this feature..

@jsmnbom

jsmnbom commented Jul 22, 2018

Copy link
Copy Markdown
Member Author

The way I see it, is that telegram is returning a custom http error code, which we should handle on par with how we handle other 4xx error codes. Adding the conflicting id is just a quality of life type of improvement in my opinion. Doesn't really harm anything, but will probably make it easier to debug issues for some people.

@jsmnbom jsmnbom added the 📋 pending-review work status: pending-review label Aug 12, 2018
tsnoam
tsnoam previously requested changes Sep 8, 2018
Comment thread telegram/error.py Outdated

class Conflict(TelegramError):
def __init__(self, msg, url):
match = re.search(r'bot(\d+):.*/', url)

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.

Can you please add a comment on the code itself what is the input string and why it means a conflicting bot id? (in favour of future us who won't remember all the details...)

@Eldinnie

Eldinnie commented Oct 2, 2018

Copy link
Copy Markdown
Member

What's the status on this? We discussed it a few times. I seem to remember we decided not to add this except for the exception?

@tsnoam

tsnoam commented Oct 2, 2018

Copy link
Copy Markdown
Member

What's the status on this? We discussed it a few times. I seem to remember we decided not to add this except for the exception?

Yes, exactly. @jsmnbom was on it. I thought it was already merged.

@jsmnbom

jsmnbom commented Oct 2, 2018

Copy link
Copy Markdown
Member Author

Yes, I'm on it, will update the PR tomorrow when I have time :)

@jsmnbom jsmnbom changed the title Add conflicting bot id to conflict error message. Add Conflict error (HTTP error code 409) Oct 29, 2018
@jsmnbom
jsmnbom dismissed tsnoam’s stale review October 29, 2018 21:08

Outdated + discussed in internal chat.

@jsmnbom
jsmnbom merged commit c9630ee into master Oct 29, 2018
@jsmnbom
jsmnbom deleted the fix-1143 branch February 14, 2019 12:53
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

📋 pending-review work status: pending-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show conflicting bot id in error when bot token is used multiple times

3 participants