Refactor #1142
closed
MetadataFileContext in pulp and pulp_rpm
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.
- 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.
- Parent issue set to #1683
- Sprint/Milestone set to 19
- Status changed from NEW to ASSIGNED
- Assignee set to semyers
/me puts on a deerstalker cap and begins to investigate
- Sprint/Milestone changed from 19 to 20
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.
Ah, and I forgot to mention that this change will require two PRs, one for platform and one for RPM.
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 :(
- Status changed from ASSIGNED to POST
- Status changed from POST to MODIFIED
- % Done changed from 0 to 100
- 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.
- 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.
- Sprint/Milestone deleted (
20)
Unsetting the sprint since it is not currently being worked on.
- Status changed from NEW to CLOSED - WONTFIX
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.
Also available in: Atom
PDF
.
Refector to use MetadataFileContext from platform
fixes #1142 https://pulp.plan.io/issues/1142