Refactor #1142
closed
MetadataFileContext in pulp and pulp_rpm
100%
Description
I discovered, there are two classes MetadataFileContext. One in pulp_rpm/plugins/pulp_rpm/plugins/distributors/yum/metadata/metadata.py and the other in pulp/server/pulp/plugins/util/metadata_writer.py. For updateinfo, comps and repomd is used one from pulp_rpm and for primary, filelists and other(fast-forward content) is used one from pulp. But point is, those two classes are almost the same. There're only minor differences (One has _write_root_tag_open in initialize, another hasn't. One has exclude for REPOMD_FILE_NAME, another hasn't. One has empty write_xml_header, another hasn't). From my point of view, it looks like one of the classes should be obsolete. But actually both are in use.
Current situation is quite confusing and makes pulp hard to debug. I suggest, one of classes should inherits from the other.
Updated by jortel@redhat.com almost 9 years ago
- Tracker changed from Issue to Refactor
- Priority changed from Normal to Low
- Groomed set to No
- Sprint Candidate set to No
Investigate to see if the class in pulp_rpm can be removed and the plugin refactored to use the one in platform. If this is the case, change the project on this issue to PULP_RPM.
Updated by semyers about 8 years ago
- Status changed from NEW to ASSIGNED
- Assignee set to semyers
/me puts on a deerstalker cap and begins to investigate
Updated by semyers almost 8 years ago
Since both classes are actually in-use, and also both have subclasses, I'm bringing the newer behavior from pulp_rpm back into platform and making the pulp_rpm version be a subclass of it that contains the tiny slice of yum-specific behavior that has been added to pulp_rpm.
It looks trivial to maintain backward-compatibility, so existing calls to this class and its subclasses in pulp_rpm won't need to be modified to adapt to the changes I'm making.
In addition to repetition in these two classes, many tests have been duplicated. I'm also going through an auditing those to similarly DRY things up.
Updated by semyers almost 8 years ago
Ah, and I forgot to mention that this change will require two PRs, one for platform and one for RPM.
Updated by semyers almost 8 years ago
semyers wrote:
In addition to repetition in these two classes, many tests have been duplicated. I'm also going through an auditing those to similarly DRY things up.
Naturally, this is the part that's taking the most time :(
Updated by semyers almost 8 years ago
- Status changed from ASSIGNED to POST
Platform PR:
https://github.com/pulp/pulp/pull/2565
RPM branch that can be a PR once the platform PR is merged:
https://github.com/pulp/pulp_rpm/compare/master...seandst:rm1142
Added by semyers almost 8 years ago
Updated by semyers almost 8 years ago
- Status changed from POST to MODIFIED
- % Done changed from 0 to 100
Applied in changeset pulp_rpm:eb322da7f849cd4bb9fa342149a998c8fe2265a6.
Updated by semyers almost 8 years ago
- Status changed from MODIFIED to ASSIGNED
My changes broke master, so I'll need to tone this down a little bit. It looked like changing some method 'pass'es to raising a notimplementederror was particularly rude.
Updated by semyers almost 8 years ago
- Status changed from ASSIGNED to NEW
- Assignee deleted (
semyers)
Sorry to drop this, but I don't think I'll be able to get to it in the immediate future.
These changes passed unit tests but broke pulp-smash, so some more work is needed to consolidate the common behavior into platform.
Updated by bmbouter over 7 years ago
- Sprint/Milestone deleted (
20)
Unsetting the sprint since it is not currently being worked on.
Updated by bmbouter about 5 years ago
- Status changed from NEW to CLOSED - WONTFIX
Updated by bmbouter about 5 years ago
Pulp 2 is approaching maintenance mode, and this Pulp 2 ticket is not being actively worked on. As such, it is being closed as WONTFIX. Pulp 2 is still accepting contributions though, so if you want to contribute a fix for this ticket, please reopen or comment on it. If you don't have permissions to reopen this ticket, or you want to discuss an issue, please reach out via the developer mailing list.
.
Refector to use MetadataFileContext from platform
fixes #1142 https://pulp.plan.io/issues/1142