Visitar URL original
Allow to hook load and save functions to use external session storage… by ezdev128 · Pull Request #36 · pyrogram/pyrogram · GitHub
Skip to content
This repository was archived by the owner on Dec 23, 2024. It is now read-only.

Allow to hook load and save functions to use external session storage… #36

Closed
ezdev128 wants to merge 2 commits into from
Closed

Allow to hook load and save functions to use external session storage… #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b37046
Allow to hook load and save functions to use external session storage…
ezdev128 Mar 18, 2018
57cea3d
Moved load session under native loading
ezdev128 Mar 19, 2018
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
2 changes: 1 addition & 1 deletion pyrogram/client/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
# along with Pyrogram. If not, see <http://www.gnu.org/licenses/>.

from .chat_action import ChatAction
from .client import Client
from .client import Client, ApiKey, Proxy

@delivrance delivrance Mar 19, 2018

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.

I saw your Pyroredis repo, and there is no need to store the proxy settings in a session.
A session should not depend on these information, users should be free to change their proxies regardless of the session used. The same applies for the api key (although I didn't see you using it)

@ezdev128 ezdev128 Mar 19, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The proxy settings is not stored into the redis session. Please re-check the code again.

@delivrance delivrance Mar 20, 2018

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.

I thought you were only dealing with sessions, and can't understand why you need Proxy (and ApiKey) to be available there. There is a different method that takes care of loading proxy and api key settings (load_config) and has nothing to do with load_session.

Maybe the auth key creation process inside load_session was confusing you a bit, if that's so we should find a nice way to move it from there. Creating a Session (and auth keys) should only be Client's job, and loading a session must happen before that.

@delivrance delivrance Mar 20, 2018 •
edited
Loading

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.

What I'm thinking is returning False from load_session in case a session does not exist and the Client needs to create an auth key, this way we can move self.auth_key = Auth(...) outside.

@ezdev128 ezdev128 Mar 20, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

During the modification I had several objectives:

  1. External shared session storage in case of working > 1 of clients (like bot-mode) with the same account credentials.
  2. I had a problem with modification session prior to start() because start() overwrites self.session with own self.session = Session(..). So I had to hook it to modify session before self.session.start().

I think will be better if those two actions (session construction and session start) will be in separate methods, like:

	def create_session(self):
		self.session = Session(
			self.dc_id,
			self.test_mode,
			self.proxy,
			self.auth_key,
			self.api_key.api_id,
			client=self
		)

	def start(self):
		self.load_config()
		self.load_session(self.session_name)
		if not self.session:
			self.create_session()
		...
		self.session.start()
		...

(or even move self.load_session() under create_session()), so it is possible to do something like:

client = Client(session_name='session_name', api_key=(api_id, api_hash))
client.create_session()
client.session.APP_VERSION = "PyRoGrAm"
...
client.start()

from .parse_mode import ParseMode
from .emoji import Emoji
30 changes: 19 additions & 11 deletions pyrogram/client/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ def __init__(self,

self.download_queue = Queue()

self.load_session_hook = None
self.save_session_hook = None

def start(self):
"""Use this method to start the Client after creating it.
Requires no parameters.
Expand All @@ -183,16 +186,18 @@ def start(self):
:class:`pyrogram.Error`
"""
self.load_config()
self.load_session(self.session_name)

self.session = Session(
self.dc_id,
self.test_mode,
self.proxy,
self.auth_key,
self.api_key.api_id,
client=self
)
if callable(self.load_session_hook):
self.load_session_hook(self)
else:
self.load_session(self.session_name)

@delivrance delivrance Mar 19, 2018 •
edited
Loading

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.

Not sure if you indented it wrongly, but this piece of code is always needed:

self.session = Session(...)

Here is where Pyrogram creates a new connection to the server.

self.session = Session(
self.dc_id,
self.test_mode,
self.proxy,
self.auth_key,
self.api_key.api_id,
client=self
)

self.session.start()

Expand All @@ -202,7 +207,10 @@ def start(self):
else:
self.authorize_bot()

self.save_session()
if callable(self.save_session_hook):
self.save_session_hook(self)
else:
self.save_session()

if self.token is None:
self.get_dialogs()
Expand Down