SG-33297 WIP #1032

julien-lang · 2025-06-18T22:10:52Z

This pull request introduces several updates to the authentication and logging modules, focusing on improving caching mechanisms, refining user interface behaviors, and enhancing logging functionality. The changes streamline the handling of user authentication data, optimize caching for YAML files, and improve logging output formatting. Additionally, there are adjustments to UI elements and method logic in the login dialog to provide a smoother user experience.

Authentication Updates

python/tank/authentication/defaults_manager.py: Implemented caching for host and login values to avoid redundant computations and added logic to reset login when host is updated. [1] [2] [3] [4]
python/tank/authentication/session_cache.py: Introduced a caching mechanism for YAML file content with a timeout of 5 minutes to improve performance. Updated session data retrieval logic to remove unnecessary None returns and clarified debug messages for missing authentication info. [1] [2] [3]

UI Enhancements

python/tank/authentication/login_dialog.py: Replaced _update_ui_according_to_site_support with _on_site_changed for better site change handling. Added a spinner to indicate site information retrieval and adjusted method logic to prevent redundant UI updates. [1] [2] [3] [4]

Logging Improvements

python/tank/log.py: Added a reset_logger method to allow resetting the logger instance and updated the log formatter to include timestamps for better traceability. [1] [2]

Code Cleanup and Clarifications

python/tank/authentication/shotgun_authenticator.py: Added TODO comments to clarify misleading method names and descriptions. [1] [2]
python/tank/authentication/site_info.py: Added TODO comments to suggest emitting signals for site info updates and consuming cache more effectively.This pull request introduces several changes across multiple files to improve caching mechanisms, streamline authentication flows, and enhance logging functionality. Key updates include the addition of caching for YAML file contents, modifications to host and login handling in the defaults manager, and adjustments to logging formats and methods.

Caching Improvements:

python/tank/authentication/session_cache.py: Introduced _YML_CONMTENT_CACHE for caching YAML file contents with a timeout mechanism to reduce redundant file reads. Added logic to clear cache entries when outdated. [1] [2]

Authentication Flow Enhancements:

python/tank/authentication/defaults_manager.py: Improved handling of host and login values by introducing _host and _login attributes with default False values, ensuring consistent initialization and caching. Updated methods such as get_host and get_login to use these attributes for better performance. [1] [2] [3]
python/tank/authentication/login_dialog.py: Refactored site change handling in _on_site_changed and _toggle_web methods to prevent unnecessary updates and improve site selection logic. Deprecated unused methods and added more descriptive logging. [1] [2]

Logging Enhancements:

python/tank/log.py: Added a reset_logger method to allow resetting the singleton logger instance. Updated the log formatter in initialize_custom_handler to include timestamps for better traceability. [1] [2]

Miscellaneous Fixes:

python/tank/authentication/session_cache.py: Updated method descriptions and logging statements for clarity, including changes to get_session_data and get_current_host. [1] [2]
python/tank/authentication/shotgun_authenticator.py: Added TODO comments to highlight misleading method names and descriptions for future refactoring. [1] [2]

hound · 2025-06-18T22:10:57Z

python/tank/authentication/defaults_manager.py

        self._user_settings = UserSettings()
        self._system_settings = SystemSettings()
        self._fixed_host = fixed_host
+        self._host = False # current value might be None


Black would make changes.

Copilot

Pull Request Overview

This PR enhances performance and usability by introducing content caching, streamlining authentication defaults handling, and improving logging and UI flows.

Added a YAML content cache with automatic invalidation to reduce redundant file reads.
Cached default host and login values in the defaults manager for faster lookups.
Extended the logger with a reset method, timestamped log output, and refactored site-change and login dialog behaviors.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
python/tank/log.py	Added `reset_logger`, adjusted formatter to include timestamps.
python/tank/authentication/site_info.py	Left placeholder TODOs and commented-out debug/sleep code.
python/tank/authentication/shotgun_authenticator.py	Inserted unprefixed TODO lines in `get_default_user`.
python/tank/authentication/session_cache.py	Introduced `_YML_CONMTENT_CACHE` with timeout-based invalidation.
python/tank/authentication/login_dialog.py	Refactored `_on_site_changed`, initialized spinner, early return in `_toggle_web`.
python/tank/authentication/defaults_manager.py	Cached `_host` and `_login` values, updated getters/setters.

Comments suppressed due to low confidence (1)

python/tank/authentication/shotgun_authenticator.py:235

Uncommented TODO line causes syntax error; prefix with # or move into a proper comment or docstring.

        TODO this method's name and description is not accurate. This method tries to return the current user with credentials. ...

Copilot · 2025-06-19T15:01:44Z

python/tank/log.py


            # example: [DEBUG tank.log] message message
-            formatter = logging.Formatter("[%(levelname)s %(name)s] %(message)s")
+            formatter = logging.Formatter("%(asctime)s [%(levelname)s] %(message)s")


The new log formatter omits the logger name, which can make identifying the source module harder; consider including %(name)s in the format string.

Suggested change

formatter = logging.Formatter("%(asctime)s [%(levelname)s] %(message)s")

formatter = logging.Formatter("%(asctime)s [%(levelname)s] %(name)s: %(message)s")

Copilot · 2025-06-19T15:01:44Z

python/tank/authentication/site_info.py

+    # logger.info("Sleep for 10s")
+    # time.sleep(30)
+    # logger.info("done sleeping")
+


[nitpick] Remove or clarify commented-out debug code and stray TODO comments to improve readability and maintainability.

Suggested change

# logger.info("Sleep for 10s")

# time.sleep(30)

# logger.info("done sleeping")

Copilot · 2025-06-19T15:01:45Z

python/tank/authentication/session_cache.py

+_YML_CONMTENT_CACHE = {}
+_YML_CONMTENT_CACHE_TIMEOUT = 300 # 5 minutes
+
 def _try_load_yaml_file(file_path):
+    global _YML_CONMTENT_CACHE
+    global _YML_CONMTENT_CACHE_TIMEOUT
+
+    cache = _YML_CONMTENT_CACHE.get(file_path, None)
+    if cache and (time.time() - _YML_CONMTENT_CACHE_TIMEOUT > cache.get("timeout", 0)):
+        try:
+            del(_YML_CONMTENT_CACHE[file_path])


Typo in constant name _YML_CONMTENT_CACHE; 'CONMTENT' should be 'CONTENT' for clarity.

Suggested change

_YML_CONMTENT_CACHE = {}

_YML_CONMTENT_CACHE_TIMEOUT = 300 # 5 minutes

def _try_load_yaml_file(file_path):

global _YML_CONMTENT_CACHE

global _YML_CONMTENT_CACHE_TIMEOUT

cache = _YML_CONMTENT_CACHE.get(file_path, None)

if cache and (time.time() - _YML_CONMTENT_CACHE_TIMEOUT > cache.get("timeout", 0)):

try:

del(_YML_CONMTENT_CACHE[file_path])

_YML_CONTENT_CACHE = {}

_YML_CONTENT_CACHE_TIMEOUT = 300 # 5 minutes

def _try_load_yaml_file(file_path):

global _YML_CONTENT_CACHE

global _YML_CONTENT_CACHE_TIMEOUT

cache = _YML_CONTENT_CACHE.get(file_path, None)

if cache and (time.time() - _YML_CONTENT_CACHE_TIMEOUT > cache.get("timeout", 0)):

try:

del(_YML_CONTENT_CACHE[file_path])

Copilot · 2025-06-19T15:01:45Z

python/tank/authentication/login_dialog.py

            )

    def _toggle_web(self, method_selected=None):
+        return


Early return at the start of _toggle_web prevents any logic from running, making the method effectively dead code; remove or adjust this return.

Suggested change

return

otherwise, create this change: -from . import QtCore +from tank.platform.qt import QtCore

uic script said: > Warning: The name 'verticalLayout_2' (QVBoxLayout) is already in > use, defaulting to 'verticalLayout_21'.

hound · 2025-06-24T15:53:11Z

python/tank/authentication/login_dialog.py

+            self.site_info = site_info
+
+
+        # what if url is empty or site_info is empty???


too many blank lines (2)

hound · 2025-06-24T15:53:11Z

python/tank/authentication/site_info.py

+
+    return INFOS_CACHE[url][1]
+
+def _retrieve_site_info(url, http_proxy=None):


expected 2 blank lines, found 1

hound · 2025-06-24T15:53:11Z

python/tank/authentication/site_info.py

+INFOS_CACHE = {}
+
+
+def get(url: str, http_proxy: str|None=None, cache_only=False) -> SiteInfo|None:


missing whitespace around bitwise or shift operator
missing whitespace around parameter equals

hound · 2025-06-24T15:53:11Z

python/tank/authentication/site_info.py

        return self._infos.get("authentication_app_session_launcher_enabled", False)
+
+# Cache the servers infos for 30 seconds.
+INFOS_CACHE_TIMEOUT = 30


expected 2 blank lines after class or function definition, found 1

hound · 2025-06-24T15:53:11Z

python/tank/authentication/site_info.py

-        self._infos = {}
-
-    def reload(self, url, http_proxy=None):
+class SiteInfo:


Black would make changes.
expected 2 blank lines, found 1

hound · 2025-06-24T17:54:45Z

python/tank/authentication/login_dialog.py

+            # guess here.
+            prev_selected_method = session_cache.get_preferred_method(site)
+        else:
+            prev_selected_method = None


local variable 'prev_selected_method' is assigned to but never used

hound · 2025-06-24T17:54:45Z

python/tank/authentication/login_dialog.py

            )

-    def _toggle_web(self, method_selected=None):
+    def _update_login_page_ui(self, auth_method: int|None=None):


missing whitespace around bitwise or shift operator
missing whitespace around parameter equals

hound · 2025-11-01T00:44:10Z

python/tank/authentication/login_dialog.py

+        self._query_task.finished.connect(lambda: logger.debug("Thread finished"))
+
+        self._query_task.start()
+


blank line contains whitespace

hound · 2025-11-01T00:44:10Z

python/tank/authentication/login_dialog.py

+
+        # Make sure signals are delivered to main thread
+        self._query_task.finished.connect(lambda: logger.debug("Thread finished"))
+


blank line contains whitespace

hound · 2025-11-01T00:44:10Z

python/tank/authentication/login_dialog.py

+        # Connect signals for this new thread
+        self._query_task.succeed.connect(self._on_site_info_response)
+        self._query_task.failed.connect(self._on_site_info_failure)
+


blank line contains whitespace

hound · 2025-11-01T00:44:11Z

python/tank/authentication/login_dialog.py

+            http_proxy=self._http_proxy,
+            parent=self
+        )
+


blank line contains whitespace

hound · 2025-11-01T00:44:11Z

python/tank/authentication/login_dialog.py

+        super().__init__(parent=parent)
+        self._url = url
+        self._http_proxy = http_proxy
+


Black would make changes.
blank line contains whitespace

hound bot reviewed Jun 18, 2025

View reviewed changes

julien-lang requested a review from Copilot June 18, 2025 22:11

This comment was marked as outdated.

Sign in to view

julien-lang added 6 commits June 19, 2025 07:54

TMP DEBUG

dc70a09

Add cache to session data to reduce the amount of logs and file loading

517d3d6

Improve docs and logs of get_session_data method

4500beb

Remove duplicated log

646986f

Move thread wait out of the __init__

1a5cf2d

No period at the end of a log line

29f732e

julien-lang force-pushed the ticket/SG-33297-review-authentication-ui-flow branch from 5d87509 to 0d61453 Compare June 19, 2025 14:58

julien-lang requested a review from Copilot June 19, 2025 14:58

Copilot AI reviewed Jun 19, 2025

View reviewed changes

julien-lang added 5 commits June 20, 2025 13:27

Remove useless logs

8d9f9b9

Cache DefaultManager host and login to prevent multiple call/logs

de8ad90

Set the timer interval once and for all

7d8d625

Fixup missing

a7da0e6

otherwise, create this change: -from . import QtCore +from tank.platform.qt import QtCore

Fixup verticalLayout_2 name was used twice

42d73db

uic script said: > Warning: The name 'verticalLayout_2' (QVBoxLayout) is already in > use, defaulting to 'verticalLayout_21'.

julien-lang force-pushed the ticket/SG-33297-review-authentication-ui-flow branch from 8c9e895 to b6d084d Compare June 20, 2025 20:57

julien-lang added 5 commits June 20, 2025 14:00

WIP

30854fc

Spinner step1: add file

2cdd77e

Spinner step2: rebuild resource file

f113977

Spinner step3: Add a label in the UI to display the spinner icon

a2ee46b

WIP add spinner and raise questions

e41f258

julien-lang force-pushed the ticket/SG-33297-review-authentication-ui-flow branch from b6d084d to e41f258 Compare June 20, 2025 21:22

julien-lang added 2 commits June 24, 2025 07:57

More changes and tests

9c7d224

eeeee

2db4e70

hound bot reviewed Jun 24, 2025

View reviewed changes

wewewerrewerw

96fdf02

hound bot reviewed Jun 24, 2025

View reviewed changes

Loading its working after a correct domain in site field

6f2efcc

hound bot reviewed Nov 1, 2025

View reviewed changes

	formatter = logging.Formatter("%(asctime)s [%(levelname)s] %(message)s")
	formatter = logging.Formatter("%(asctime)s [%(levelname)s] %(name)s: %(message)s")

	# logger.info("Sleep for 10s")
	# time.sleep(30)
	# logger.info("done sleeping")

		self.site_info = site_info


		# what if url is empty or site_info is empty???


		return INFOS_CACHE[url][1]

		def _retrieve_site_info(url, http_proxy=None):

		INFOS_CACHE = {}


		def get(url: str, http_proxy: str\|None=None, cache_only=False) -> SiteInfo\|None:

		self._query_task.finished.connect(lambda: logger.debug("Thread finished"))

		self._query_task.start()


		# Make sure signals are delivered to main thread
		self._query_task.finished.connect(lambda: logger.debug("Thread finished"))

SG-33297 WIP #1032

Are you sure you want to change the base?

SG-33297 WIP #1032

Uh oh!

Conversation

julien-lang commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Authentication Updates

UI Enhancements

Logging Improvements

Code Cleanup and Clarifications

Caching Improvements:

Authentication Flow Enhancements:

Logging Enhancements:

Miscellaneous Fixes:

Uh oh!

hound bot Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

hound bot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

julien-lang commented Jun 18, 2025 •

edited

Loading