feat: export video metadata#905
Conversation
export a ganymede.json video metadata file
Summary of ChangesHello @Zibbp, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new feature that allows for the export of detailed video metadata. This functionality is crucial for backup purposes and for facilitating the migration of video data to other instances. The changes encompass both frontend and backend, providing a user-triggerable and automatically scheduled mechanism to generate structured JSON files containing video information and its related entities. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to export video metadata as a JSON file. The implementation includes a new background task, UI elements on the admin page, and DTO conversions for various entities. The changes are well-structured. My review includes suggestions to improve performance by batching database queries in the new worker, enhance code clarity and consistency in function signatures and data structures, and address a minor style issue. I've also noted a point about the use of the copier library, suggesting manual mapping for better performance and safety.
| vods, err := store.Client.Vod.Query().Select(entVod.FieldID).All(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to fetch videos: %w", err) | ||
| } | ||
|
|
||
| for _, v := range vods { | ||
| err := vodService.ExportMetadata(ctx, v) | ||
| if err != nil { | ||
| logger.Error().Err(err).Msgf("failed to export metadata for video %s", v.ID) | ||
| continue | ||
| } | ||
| logger.Debug().Msgf("exported metadata for video %s", v.ID) | ||
| } |
There was a problem hiding this comment.
This implementation fetches all VODs from the database at once. If there is a large number of videos, this could lead to high memory consumption and potentially cause the task to exceed its 5-minute timeout. It would be more robust to process the videos in batches. A similar batching pattern is used in UpdateVideoStorageUsageWorker.
const batchSize = 100
offset := 0
for {
vods, err := store.Client.Vod.Query().
Select(entVod.FieldID).
Limit(batchSize).
Offset(offset).
All(ctx)
if err != nil {
return fmt.Errorf("failed to fetch videos: %w", err)
}
if len(vods) == 0 {
break
}
for _, v := range vods {
err := vodService.ExportMetadata(ctx, v)
if err != nil {
logger.Error().Err(err).Msgf("failed to export metadata for video %s", v.ID)
continue
}
logger.Debug().Msgf("exported metadata for video %s", v.ID)
}
offset += batchSize
}| func (s *Service) ExportMetadata(ctx context.Context, video *ent.Vod) error { | ||
| vod, err := s.Store.Client.Vod. | ||
| Query(). | ||
| Where(entVod.IDEQ(video.ID)). |
There was a problem hiding this comment.
The function ExportMetadata takes video *ent.Vod as an argument, but it only uses video.ID to immediately re-query the database for the full VOD object. It would be clearer and more explicit to change the function signature to accept a videoID uuid.UUID directly. This makes the function's dependency explicit. You'll also need to update the call site in internal/tasks/periodic/periodic.go.
| func (s *Service) ExportMetadata(ctx context.Context, video *ent.Vod) error { | |
| vod, err := s.Store.Client.Vod. | |
| Query(). | |
| Where(entVod.IDEQ(video.ID)). | |
| func (s *Service) ExportMetadata(ctx context.Context, videoID uuid.UUID) error { | |
| vod, err := s.Store.Client.Vod. | |
| Query(). | |
| Where(entVod.IDEQ(videoID)). |
1 participant
Add a task to export the Ganymede metadata of a video. This is stored in a non-standard (not following storage templates) file as this is meant to be a special "metadata" file not part of the archive and exclusive to Ganymede.
Still need to figure out versioning and how it would work with future or modified fields.