[ENG-6539] Fixed archiving gv addons #10851
Conversation
| include_path = f'external_{gv_account_data.resource_type.split('-')[1]}_service', | ||
| service_wb_key = gv_account_data.get_included_attribute( | ||
| include_path=include_path, | ||
| attribute_name='external_service_name' if addon_type == AddonType.CITATION else 'wb_key' |
There was a problem hiding this comment.
This is to consistent with current stage configuration. If wb_key for citation addons is configured properly, we may replace it with
| attribute_name='external_service_name' if addon_type == AddonType.CITATION else 'wb_key' | |
| attribute_name='wb_key' |
There was a problem hiding this comment.
Will we also need to do this for computing addons?
There was a problem hiding this comment.
If we set wb_key in GV admin, we won't
| include_path = ('base_account', f'external_{addon_type}_service') | ||
| service_wb_key = gv_addon_data.get_included_attribute( | ||
| include_path=include_path, | ||
| attribute_name='external_service_name' if addon_type == AddonType.CITATION else 'wb_key' |
There was a problem hiding this comment.
This is to consistent with current stage configuration. If wb_key for citation addons is configured properly, we may replace it with
| attribute_name='external_service_name' if addon_type == AddonType.CITATION else 'wb_key' | |
| attribute_name='wb_key' |
| 'operation_kwargs': {}, | ||
| 'operation_name': 'list_collection_items', | ||
| 'operation_kwargs': { | ||
| 'collection_id': addon['attributes']['root_folder'] if list_id == 'ROOT' else list_id |
There was a problem hiding this comment.
I've noticed the issue when actual root items would be fetched instead if child items of configured root. This resolves it
@adlius fyi
There was a problem hiding this comment.
I think I fixed this issue in another PR. What I did is if there is a list_id I would always invoke list_collection_items. The Pr is here.
| def _initialize_ephemeral_storage_node_settings(): | ||
| from addons.base.models import BaseStorageAddon | ||
|
|
||
| global _StorageEphemeralNodeSettings | ||
|
|
||
| class StorageEphemeralNodeSettings(EphemeralNodeSettings, BaseStorageAddon): | ||
| pass | ||
|
|
||
| _StorageEphemeralNodeSettings = StorageEphemeralNodeSettings |
There was a problem hiding this comment.
I'm not happy with this approach either, so I am more than welcome to suggestions
There was a problem hiding this comment.
I'm afraid I don't have any other suggestions.
There was a problem hiding this comment.
Might avoid using a global with this:
def _initialize_ephemeral_storage_node_settings():
if not hasattr(_initialize_ephemeral_storage_node_settings, "_storage_node_settings"):
from addons.base.models import BaseStorageAddon
class StorageEphemeralNodeSettings(EphemeralNodeSettings, BaseStorageAddon):
pass
_initialize_ephemeral_storage_node_settings._storage_node_settings = StorageEphemeralNodeSettings
return _initialize_ephemeral_storage_node_settings._storage_node_settings
_StorageEphemeralNodeSettings = _initialize_ephemeral_storage_node_settings()
I think that cover any import problems with a global.
| def _initialize_ephemeral_storage_node_settings(): | ||
| from addons.base.models import BaseStorageAddon | ||
|
|
||
| global _StorageEphemeralNodeSettings | ||
|
|
||
| class StorageEphemeralNodeSettings(EphemeralNodeSettings, BaseStorageAddon): | ||
| pass | ||
|
|
||
| _StorageEphemeralNodeSettings = StorageEphemeralNodeSettings |
There was a problem hiding this comment.
I'm afraid I don't have any other suggestions.
| include_path = f'external_{gv_account_data.resource_type.split('-')[1]}_service', | ||
| service_wb_key = gv_account_data.get_included_attribute( | ||
| include_path=include_path, | ||
| attribute_name='external_service_name' if addon_type == AddonType.CITATION else 'wb_key' |
There was a problem hiding this comment.
Will we also need to do this for computing addons?
There was a problem hiding this comment.
I have some other PRs that I made that fixed some of the issues. I think you might want to take a look at those:
https://github.com/CenterForOpenScience/gravyvalet/pull/178/files
https://github.com/CenterForOpenScience/osf.io/pull/10837/files
| 'operation_kwargs': {}, | ||
| 'operation_name': 'list_collection_items', | ||
| 'operation_kwargs': { | ||
| 'collection_id': addon['attributes']['root_folder'] if list_id == 'ROOT' else list_id |
There was a problem hiding this comment.
I think I fixed this issue in another PR. What I did is if there is a list_id I would always invoke list_collection_items. The Pr is here.
| include_path = f'external_{gv_account_data.resource_type.split('-')[1]}_service', | ||
| service_wb_key = gv_account_data.get_included_attribute( | ||
| include_path=include_path, | ||
| attribute_name='external_service_name' if addon_type == AddonType.CITATION else 'wb_key' |
Purpose
Changes
QA Notes
Please make verification statements inspired by your code and what your code touches.
What are the areas of risk?
Any concerns/considerations/questions that development raised?
Documentation
Side Effects
Ticket