Skip to content
This repository was archived by the owner on Dec 23, 2024. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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()

from .parse_mode import ParseMode
from .emoji import Emoji
30 changes: 19 additions & 11 deletions pyrogram/client/client.py
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)
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.

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