Skip to content

Tags refactor #1250

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

Merged
merged 30 commits into from
May 6, 2025
Merged

Tags refactor #1250

merged 30 commits into from
May 6, 2025

Conversation

tomchop
Copy link
Collaborator

@tomchop tomchop commented May 3, 2025

This PR tackles #1165. Essentially:

  • Removing the Tag Graph
  • Replacing it with tags embedded in the object
  • Changin the structure of the .tags attribute to be a list instead of a dict.
  • Deprecate the Tag Graph
  • Replace with embedded tags
  • Fix everything that's broken
  • Test UI
  • Remove old indexes / collections (tentative, might be worth keeping for a migration or two)
  • Migration scripts
  • Add tag indexes
  • Test migration script on a live DB

BREAKING CHANGES

  • Object tags are now lists instead of dicts

@tomchop tomchop changed the title Tags Tags refactor May 3, 2025
@tomchop tomchop requested a review from Copilot May 3, 2025 12:58
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the tagging system by deprecating the Tag Graph and replacing it with embedded tags in objects. Key changes include updating tests to work with dictionary‐based tag structures, refactoring tag-related methods in the core schemas and API endpoints, and removing obsolete graph queries.

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/schemas/tag.py Updated tests use new embedded tag format and adjust expiration assertions.
tests/schemas/package.py Modified tests to iterate over dictionary values for tags.
tests/schemas/entity.py Removed tests related to the deprecated tag graph.
tests/helpers.py Added a sleep delay to assist with eventual consistency in tests.
tests/core_tests/tasks.py Changed set assertions to count assertions for tag operations.
tests/apiv2/* Updated tag handling logic to use the new embedded tag representations.
plugins/analytics/public/expire_tags.py Refactored expiration logic to use the new embedded tag system.
core/web/apiv2/* Removed obsolete graph query parameters and updated tag extraction logic.
core/schemas/tag.py Removed an in-place normalize function in favor of the one defined in model.py.
core/schemas/model.py Introduced YetiTagInstance and refactored tag methods to use embedded tags.
core/schemas/graph.py Removed the TagRelationship definition and related logic.
core/database_arango.py Removed tagging methods that relied on graph queries.

@tomchop tomchop marked this pull request as ready for review May 3, 2025 21:08
@tomchop tomchop requested review from udgover and sebdraven May 3, 2025 21:09
Copy link
Collaborator

@udgover udgover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice PR! potential typo + left some comments and questions about tag function implementation.

@tomchop tomchop requested a review from udgover May 6, 2025 11:50
Copy link
Collaborator

@udgover udgover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go!

@tomchop tomchop merged commit 6a38091 into main May 6, 2025
2 checks passed
@tomchop tomchop deleted the tags branch May 6, 2025 12:19
@tomchop tomchop added enhancement breaking Changes that might break some implementations noteworthy PRs that are noteworthy / introduce new features labels May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Changes that might break some implementations enhancement noteworthy PRs that are noteworthy / introduce new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants