-
Notifications
You must be signed in to change notification settings - Fork 7
Regroup all session creation in util get_localAPI_session or APISession class. #129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 3.2.12-8.3-devel
Are you sure you want to change the base?
Changes from all commits
4c056a0
5434904
a7af036
d09dd48
b0734fa
d1d6cb5
c4d891b
bc80916
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -521,7 +521,7 @@ | |
|
|
||
| <!-- Agent database query errors 150+ --> | ||
| <code> | ||
| <name>APISession</name> | ||
| <name>ApiSession</name> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Nambrok For additional comment.
|
||
| <description>Failed to initialize XMLRPC connection</description> | ||
| <value>150</value> | ||
| </code> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -648,19 +648,10 @@ def clear(self): | |
| self.linstor = None | ||
| self.vdi_type_to_cowutil = {} | ||
|
|
||
| class Load: | ||
| def __init__(self, session): | ||
| self.session = session | ||
|
|
||
| def cleanup(self): | ||
| if self.session: | ||
| self.session.xenapi.session.logout() | ||
| self.session = None | ||
|
|
||
| def __init__(self, uri, group_name) -> None: | ||
| self._uri = uri | ||
| self._group_name = group_name | ||
| self._loads: List[MultiLinstorCowUtil.Load] = [] | ||
| self._loads: List[util.ApiSession] = [] | ||
| self._executor_data = self.ExecutorData() | ||
|
|
||
| def __del__(self): | ||
|
|
@@ -687,8 +678,7 @@ def get_local_cowutil(self, vdi_type): | |
| return instance | ||
|
|
||
| def _init_executor_thread(self): | ||
| session = util.get_localAPI_session() | ||
| load = self.Load(session) | ||
| api_session = util.ApiSession("SM-linstorvhdutil") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest to see if we can get automatically the file+function name in the session name, this will reduce maintenance in case of changes (see, for example it should be This is a suggestion but ain't blocking for me, could even be addressed in a separate PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer having an explicit session name; logs indicating a session aren't necessarily debug logs. They serve as a label that is human-readable and can be independent of the implementation. If we want to be more precise here, it would be |
||
| try: | ||
| linstor = LinstorVolumeManager( | ||
| self._uri, | ||
|
|
@@ -697,18 +687,16 @@ def _init_executor_thread(self): | |
| logger=util.SMlog | ||
| ) | ||
| self._executor_data.linstor = linstor | ||
| self._executor_data.session = session | ||
| self._executor_data.session = api_session.session | ||
| except: | ||
| self._executor_data.clear() | ||
| load.cleanup() | ||
| raise | ||
|
|
||
| self._loads.append(load) | ||
| self._loads.append(api_session) | ||
|
|
||
| def _cleanup(self): | ||
| for load in self._loads: | ||
| try: | ||
| load.cleanup() | ||
| load.logout() | ||
| except Exception as e: | ||
| util.SMlog(f"Failed to clean load executor: {e}") | ||
| self._loads.clear() | ||
Uh oh!
There was an error while loading. Please reload this page.