Skip to content

Conversation

@Eldinnie
Copy link
Member

This adds a prefix of Bot:<id>:worker: to the name of the worker threads.
Fixes #1332

@Eldinnie Eldinnie requested a review from jsmnbom February 28, 2019 14:57
@jsmnbom
Copy link
Member

jsmnbom commented Feb 28, 2019

Looks pretty good all around, got some comments though.

  1. This doesn't 100% fix give threads a name prefix #1332 does it? It only adds the prefix to worker threads, is there anything preventing us from doing the same with the dispatcher and updater thread?
  2. Should it maybe be called ptb:<id>:worker: or telegram:<id>:worker or something like that? Just to make it 100% explicit what the thread is?
  3. I'm not really a fan of having a method just for testing purposes. Can't you just do dp._Dispatcher__async_threads in the test?

@Eldinnie
Copy link
Member Author

@jsmnbom I will address your notes later, want to see if this indeed fixes Travis' behavior

@bmxp
Copy link

bmxp commented Mar 19, 2019

I am looking forward for your improvements and will try out asap ...

@jsmnbom
Copy link
Member

jsmnbom commented Apr 4, 2019

@Eldinnie any updates on this? Do you agree with my comments? I can implement them if you dont' have time ^ ^

@NickKush
Copy link

NickKush commented May 6, 2019

Any updates about this PR? I have the same issue as #1332 ( ._.)

@bmxp
Copy link

bmxp commented May 7, 2019

As long as this PR is not accepted into Release 12 I have tricked around with something like this which is not beautiful but does the necessary change:

pretty_thread_names = True
prefix_programname = "YourProgramnameHere"
prefix_worker_thread = "Worker"

# theUpdater contains the telegram Updater Object

if pretty_thread_names:
    try:
        for t in theUpdater._Updater__threads:
          if 'dispatcher' in t.name: 
            t.name = prefix_programname+' Dispatcher'
            
          if 'updater' in t.name: 
            t.name = prefix_programname+' Updater'
            
        for t in theUpdater.dispatcher._Dispatcher__async_threads:
          *_, num = t.name.split('_')
          t.name = '{0} {1} {2}'.format(prefix_programname, prefix_worker_thread, num) if num.isnumeric() else num

    except:
        pass

@Eldinnie Eldinnie changed the base branch from master to V12 June 6, 2019 11:54
Eldinnie added 2 commits June 6, 2019 13:55
This adds a prefix of `Bot:<id>:worker:` to the name of the worker threads.
Fixes #1332
@Eldinnie
Copy link
Member Author

Eldinnie commented Jun 6, 2019

Renamed all threads and changed base.

@Eldinnie
Copy link
Member Author

Eldinnie commented Jun 6, 2019

@jsmnbom care to take another look?

@Eldinnie Eldinnie self-assigned this Jun 6, 2019
@Eldinnie Eldinnie added the 📋 pending-review work status: pending-review label Jun 6, 2019
@jsmnbom
Copy link
Member

jsmnbom commented Jun 6, 2019

This mostly looks good now, but i still don't think i can see my comment 3 adressed?

  1. I'm not really a fan of having a method just for testing purposes. Can't you just do dp._Dispatcher__async_threads in the test?

@Eldinnie
Copy link
Member Author

Eldinnie commented Jun 6, 2019

No you can't.
It's a protected variable

Copy link
Member

@jsmnbom jsmnbom left a comment

Choose a reason for hiding this comment

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

Alright, if that's not possible then, this looks good :)

@Eldinnie Eldinnie merged commit edad6e8 into V12 Aug 23, 2019
@Eldinnie Eldinnie deleted the fix_1332 branch August 23, 2019 20:57
@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.

5 participants