Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbitRemoved
WalkthroughRemoved Jira assignee-to-owner mapping and related normalization/persistence logic; tests and changelog updated to stop deriving Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Jincxz
left a comment
There was a problem hiding this comment.
Mostly LGTM.
The syncing of the owner was also removed for the task convertor but I think this is fine since we no longer sync tasks from Jira to OSIDB. Could another @RedHatProductSecurity/osidb-devs confirm this?
There are also a few more places in the tests that can be cleaned up:
osidb/collectors/jiraffe/tests/test_convertors.py
Lines 394 to 395 in 4d142ef
|
@Jincxz, addressed all the cleanup suggestions. Thanks for the review! |
Jincxz
left a comment
There was a problem hiding this comment.
LGTM. Thank you for addressing my comments!
There was a problem hiding this comment.
I agree with @Jincxz that the collector is disabled in ops, but we have a scope mismatch here. OSIDB-4875 and the changelog specifically address trackers, yet this PR modifies tasks as well.
We should either:
- Update the Jira issue and changelog to explicitly include task owner removal.
- Revert the changes to task owners to keep this PR strictly about trackers.
Otherwise, we're introducing a 'silent' breaking change for tasks that isn't documented anywhere.
|
Good catch! As the task collector is disabled in ops, keeping the change. Will update the |
JakubFrejlach
left a comment
There was a problem hiding this comment.
Thank you for addressing my points! LGTM now, just squash commits a bit please.
8a99b9b to
441dcac
Compare
Remove
ownerandqe_ownerparsing from Jira tracker download.These fields used
JiraUserMapping.cloud_id_to_kerberoswhich fails for service accounts and deactivated users.-convertors.py - Removed
ownerandqe_ownerparsing from JiraTrackerConvertor and JiraTaskConvertor.-constants.py - Removed
assignee: ["owner"].-test_convertors.py - Updated tests accordingly.
-CHANGELOG.md - Added entry.
Closes: OSIDB - 4875