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 intopyrogram:masterfrom
ezdev128:master
Closed

Allow to hook load and save functions to use external session storage…#36
ezdev128 wants to merge 2 commits intopyrogram:masterfrom
ezdev128:master

Conversation

@ezdev128
Copy link

@delivrance
Copy link
Member

Thanks for this, but Pyrogram is still pretty young and doesn't even persistently cache peers yet, as it should. A redis backend will surely make more sense if there were thousands of peers to store.

if callable(self.load_session_hook):
self.load_session_hook(self)
else:
self.load_session(self.session_name)
Copy link
Member

@delivrance delivrance Mar 19, 2018

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.


from .chat_action import ChatAction
from .client import Client
from .client import Client, ApiKey, Proxy
Copy link
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)

Copy link
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.

Copy link
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.

Copy link
Member

@delivrance delivrance Mar 20, 2018

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.

Copy link
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()

@delivrance
Copy link
Member

Closing because this needs a revamp since the introduction of storage engines.

@delivrance delivrance closed this May 16, 2020
AlbertEinsteinTG pushed a commit to AlbertEinsteinTG/pyrogram that referenced this pull request Oct 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants