Skip to content
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

Added --recent support #30

Merged
merged 3 commits into from
Oct 20, 2020
Merged

Added --recent support #30

merged 3 commits into from
Oct 20, 2020

Conversation

chilinux
Copy link
Contributor

This change is intended to allow for faster incremental updates to the errata based on how recently they were issued.

For example, doing --recent=90 will only attempt to import errata which have a issued date in the XML of within 90 days of today.

The change requires perl-DateTime package be installed.

It also replaces the find_packages subroutine, the packages.get_details API call and building the name2id/name2channel hash tables.

The replacement to find_packages is find_packages_by_advisory which uses find_package_by_name. Then in find_package_by_name the API calls of packages.findByNvrea and packages.listProvidingChannels is used. These API calls are listed as supported in Spacewalk 1.4 API and still supported in Uyuni. They probably are supported by earlier versions but I can't find the API documents for version previous to 1.4.

During testing, I get the following results:

Full import - find_packages: 1 hour 55 miinutes
Full import - find_packages_by_advisory: 2 hours 9 minutes
90 day import - find_packages: 1 hour 18 minutes
90 day import - find_packages_by_advisory: 4 minutes

So, while during my own testing the change is 12% slower for a full import, it is much faster for incremental errata imports.

It should be noted that the issue date may not be the same exact day that an errata is publised in the XML file. I figured daily imports with a 90 day period should be long enough to catch previously issued but recently published errata. The change gives the user the flexability to choose their own time period with the --recent flag.

Hopefully others find this change useful or can provide feedback if I'm doing something incorrectly.

@stevemeier
Copy link
Owner

Thanks for sending in this PR.

Here are a few comments, in no specific order:

In an effort to reduce runtime, the default errata file provided on my website is already limited to the last 180 days.

I would prefer to use Date::Parse instead of DateTime. While DateTime is the superior module in terms of functionality, Date::Parse is more widely available and would keep dependencies smaller.

I am not sure if the regex in line 952 really catches everything. Did you test that?
Before committing such a major change I would like to be sure that everything still works.

Kind regards,
Steve

@chilinux
Copy link
Contributor Author

Updated to use Date::Parse instead of DateTime.

I have tested the regex of Line 952. So far it has worked with all package names that follows the naming conventions from rpmbuild which includes all of the ones from RHEL, CentOS, Fedora and EPEL. It also handles all of the existing architechures in the errata-latest.xml including x86_64, i386, i686, noarch, src, armv7hl, aarch64, ppc, ppc64 and ppc64le. However, I can change the regex to whatever you want.

I have also done a side by side comparison of the debug output using find_packages vs find_packages_by_advisory.

Did you want this applied to a test branch instead for the short term?

@stevemeier
Copy link
Owner

Ok, great. Just wanted to make sure this change doesn't break anything.
I'll still have to test it myself which I can do next week.

If tests succeed, I'll merge it into master.

@stevemeier
Copy link
Owner

Just started testing and I noticed one issue in the code.

A sub can not return two arrays (see line 1007), instead it needs to return two references (as in line 965).

Would you be able to change that? Thanks.

@chilinux
Copy link
Contributor Author

Sorry, I have been spoiled by python. Line 1007 has been corrected to be in perl. :)

@stevemeier stevemeier merged commit c55a207 into stevemeier:master Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants