Sdesk 7469 #2831
Sdesk 7469 #2831
Conversation
jerome-poisson
force-pushed
the
SDESK-7469
branch
from
1cff562 to
164a9e3
Compare
- use `validate_data_relation_async` to convert legacy `Resource.rel`. - add `LinkedInPackage` dataclass instead of generic dict.
There was a problem hiding this comment.
@jerome-poisson I have gone over the old schema and this one. There are some Elastic mapping differences between these models and the old ones. We need to make sure that the old elastic schema and new ones match, otherwise users may have search issues.
Maybe you can grab the elastic mapping of the old one, and create a unit test for this new one, to ensure the mappings are the same. Check here for an example test
superdesk/archive_async/service.py
Outdated
| mediaService = ArchiveMediaService() | ||
| cropService = CropService() | ||
|
|
||
| async def on_fetched(self, docs) -> None: |
There was a problem hiding this comment.
These on_fetched don't belong on the new AsyncResourceService as they have nothing to do with the data layer (these are used on REST APIs). They can be removed from this service (will be added to a custom REST API class later for this resource)
| for item in items: | ||
| handle_existing_data(item) | ||
|
|
||
| async def on_create(self, docs: list[ArchiveResourceModel]) -> None: |
There was a problem hiding this comment.
Is there a way to make a base class that this and the original resource service can both use? The old Eve resources now support async on_ functions (such as def on_create, def on_update etc), just not on the core method functions (such as def create, def update etc).
I've done this with the async publish endpoints to reduce the amount of code changes required. I used the package service as an example (pure python class, implementing the on_create etc methods).
And then you would update both the old and this service to use those functions.
Keeping in mind, if the class makes changes to the dictionary (especially here in creation), then you would need to propagate those changes onto the Pydantic model instance), maybe just using the clone_with function
| indexes=[ | ||
| MongoIndexOptions( | ||
| name="item_id_1", | ||
| keys=[("item_id", 1), ("background", 1)], |
There was a problem hiding this comment.
Same here, will require unique=False
superdesk/types/archive.py
Outdated
| flags: Flag | None = None | ||
| sms_message: str | None = None | ||
| format: str = FORMATS.HTML | ||
| auto_publish: bool |
There was a problem hiding this comment.
This should probably have a default of False
superdesk/types/archive.py
Outdated
|
|
||
| @dataclass | ||
| class Attachment: | ||
| pass |
There was a problem hiding this comment.
Missing attachment field, resource link to the attachments resource
superdesk/types/archive.py
Outdated
| publish_schedule: datetime | None = None | ||
| # FIXME: duplicate of ItemSchema | ||
| schedule_settings: ScheduleSettings | None = None | ||
| used: bool |
There was a problem hiding this comment.
This could probably be defaulted to False
superdesk/types/archive.py
Outdated
| # FIXME: duplicate of ItemSchema | ||
| schedule_settings: ScheduleSettings | None = None | ||
| used: bool | ||
| used_count: int |
There was a problem hiding this comment.
Could probably default to 0, what do you think?
superdesk/types/archive.py
Outdated
| used_count: int | ||
| used_updated: datetime | ||
| metrics: dict[str, Any] | ||
| _type: str |
There was a problem hiding this comment.
This is already defined in parent resource class, no need to re-define here
- disable Rest endpoint - use a `BaseContentItem` model as parent class for `MetadataResource` and `ContentAPIItem` - use `fields.Keyword` when `not_analyzed` is used in legacy resource. - added default values for many fields - removed some custom models in favor of existing ones from `content_api.items.model`
This PR is still a WIP. The classes are quite extensive, and the methods calling each other in a layered fashion make it challenging to follow. I have several questions about how to best manage certain method or type porting that I would like to tackle.