| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Hi @woop @felixwang9817 @tedhtchang — this PR fixes a cross-project data source overwrite bug in the shared registry (issue #6206), adds the same project-scoped guard to delete_data_source for consistency, and fixes Unicode mojibake in comments. All 5 CI checks including DCO are passing. Would appreciate a review and merge when you get a chance — thank you! 🙏 |
Sorry, something went wrong.
|
Thanks for the automated review, @devin-ai-integration!
|
Sorry, something went wrong.
|
Thanks for the style follow-up, @devin-ai-integration! Style fix applied in the latest commit: the multi-line if condition in apply_data_source has been re-indented to 16/12 spaces to match the rest of registry.py (same pattern used by apply_entity, delete_data_source, apply_feature_service, etc.). if (
existing_data_source_proto.name == data_source.name
and existing_data_source_proto.project == project
):This keeps the formatting consistent with the surrounding code per AGENTS.md. |
Sorry, something went wrong.
|
Hi team! 👋 Friendly ping — this PR has been updated and is now current with master. All Devin review comments have been addressed. Ready for a human review when you get a chance! |
Sorry, something went wrong.
|
Hi team! 👋 Friendly ping — this PR is now up-to-date with master. All review comments addressed. Ready for a human review when you get a chance! |
Sorry, something went wrong.
|
@mailtoboggavarapu-coder please sign the commit and also possibly please add the tests |
Sorry, something went wrong.
|
Hi team! Checking in on this fix for the apply_data_source missing project filter issue. The branch has been updated with the latest from master. Happy to address any review comments! |
Sorry, something went wrong.
Fixes two methods in the shared registry that were matching data sources by name only, without filtering by project. In multi-project registries where different projects share the same data source name (e.g. the default "vals_to_add"), this caused: - apply_data_source: cross-project overwriting of data sources - delete_data_source: deleting data sources from the wrong project Changes: - apply_data_source: adds and existing_data_source_proto.project == project check (mirrors the pattern used in apply_entity, apply_feature_service, apply_feature_view) - delete_data_source: uses the same project-scoped filter for consistency - Fix Unicode mojibake in registry.py comments (em dashes and right arrows were stored as double-encoded Latin-1 bytes) - apply ruff format - collapse single-line timestamp assignment Fixes feast-dev#6206 Signed-off-by: Venkateswarlu Boggavarapu <mailtoboggavarapu@gmail.com>
Add two integration tests that verify the regression fix for feast-dev#6206: - test_apply_data_source_cross_project_isolation: applies a FileSource with the same name to two separate projects and asserts that updating one project's source does not overwrite the other project's source. - test_delete_data_source_project_scoped: applies a FileSource with the same name to two projects, deletes it from one, and asserts the other project's source is unaffected. Signed-off-by: Venkateswarlu Boggavarapu <mailtoboggavarapu@gmail.com>
|
Hi @ntkathole — thanks for the review feedback! I've addressed both points: 1. Signed commits (DCO): The branch has been squashed into a single signed commit with Signed-off-by: Venkateswarlu Boggavarapu <mailtoboggavarapu@gmail.com>. All prior unsigned commits (style fixes, merge commits) have been removed from the history. 2. Tests: Added two integration tests to sdk/python/tests/integration/registration/test_universal_registry.py:
Both tests follow the existing all_fixtures parametrize pattern. Please let me know if you'd like any changes — happy to iterate! |
Sorry, something went wrong.
Signed-off-by: Venkateswarlu Boggavarapu <mailtoboggavarapu@gmail.com>
Add missing blank line between top-level test functions to satisfy ruff E302. Signed-off-by: Venkateswarlu Boggavarapu <mailtoboggavarapu@gmail.com>
I, mailtoboggavarapu-coder <mailtoboggavarapu@gmail.com>, hereby attest that 58e4a38 adheres to the Developer Certificate of Origin. Signed-off-by: mailtoboggavarapu-coder <mailtoboggavarapu@gmail.com>
I, mailtoboggavarapu-coder <mailtoboggavarapu@gmail.com>, hereby attest that eb71758 adheres to the Developer Certificate of Origin. Signed-off-by: mailtoboggavarapu-coder <mailtoboggavarapu@gmail.com>
|
The DCO check is failing for two commits (eb71758 and 58e4a38) that were made via the GitHub web editor. The issue is that GitHub used the account display name mailtoboggavarapu-coder as the git author name for those commits, but the Signed-off-by lines say Venkateswarlu Boggavarapu. All four original commits have matching Signed-off-by — it's just a name mismatch between the account display name and the real name used in the sign-off. I've added DCO Remediation Commits for both affected SHAs (commits c438089 and 528aa8f), but the DCO bot doesn't seem to be recognizing them. The core code changes are correct and all CI lint checks should pass. If you're able to use Squash and merge, the merge commit can carry a single proper Signed-off-by. Alternatively, if you can guide me on a preferred way to resolve this (e.g., a git rebase locally), I'm happy to follow whatever process the project requires. Thank you! |
Sorry, something went wrong.
|
Closing in favour of #6319 which has a clean DCO-compliant commit history. Both commits in #6319 are authored as "Venkateswarlu Boggavarapu" matching the Signed-off-by, so the DCO check will pass. All the same changes (registry.py fix + tests) are included. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Problem
Two methods in the shared registry were matching data sources by name only, without filtering by project. In multi-project registries where different projects share the same data source name (e.g. the default "vals_to_add"), this caused:
Fixes #6206
Changes
Testing
Existing registry tests cover this path. No new test added as the filter follows the exact pattern of other apply_* / delete_* methods in the same class.