red-hat-storage / errata-tool

Modern Python API to Red Hat's Errata Tool
MIT License
31 stars 34 forks source link

documentation error: e.metadataCdnRepos API requires a list of strings, not a single string #222

Closed nickboldt closed 2 years ago

nickboldt commented 3 years ago

According to the docs on https://pypi.org/project/errata-tool/ this should work:

from errata_tool import Erratum

e = Erratum(errata_id=82818)
assert 'docker' in e.content_types
e.metadataCdnRepos(enable='rhel-7-server-crw-text-only-rpms__x86_64')

And this URL works:

https://errata.devel.redhat.com/api/v1/erratum/82818/metadata_cdn_repos

As does this page:

https://errata.devel.redhat.com/errata/docker_cdn_repos/82818

But I cannot run the above script, as I get this error:

Traceback (most recent call last):
  File "errataWork.py", line 19, in <module>
    e.metadataCdnRepos(enable='rhel-7-server-crw-text-only-rpms__x86_64')
  File "/home/nboldt/.local/lib/python3.7/site-packages/errata_tool/erratum.py", line 372, in metadataCdnRepos
    return self._cdn_repos('metadata_cdn_repos', enable, disable)
  File "/home/nboldt/.local/lib/python3.7/site-packages/errata_tool/erratum.py", line 414, in _cdn_repos
    result.raise_for_status()
  File "/usr/lib/python3.7/site-packages/requests/models.py", line 940, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://errata.devel.redhat.com/api/v1/erratum/82818/metadata_cdn_repos

This also does not work:

assert e.text_only
e.textOnlyRepos(enable='rhel-7-server-crw-text-only-rpms__x86_64')
Traceback (most recent call last):
  File "errataWork.py", line 20, in <module>
    assert e.text_only
AssertionError

Not sure what I'm doing wrong, or if this is a bug... or perhaps Errata is confused because the CDN is "text only" but the errata contains docker images?

ktdreyer commented 2 years ago

The Errata Tool has a few different types of advisories, and a "text-only" advisory is different than a "Docker" (ie container) advisory.

rhel-7-server-crw-text-only-rpms__x86_64 has the word "text-only" in it, so it's surprising to me that you would expect to enable a "text-only" CDN repo for your container advisory. Has that worked in the past? My experience leads me to think this might be a release-engineering mis-configuration.

ktdreyer commented 2 years ago

Another thing that's surprising here is that https://errata.devel.redhat.com/advisory/82818 has "rhel8" based containers, and this text-only CDN repo has "rhel-7" in the name.

nickboldt commented 2 years ago

@Chris O'Brien @.***> Any suggestions for what to do here? We've been using the same "text only rhel7" advisory thingy for >2yrs, and we no longer have any rhel7 containers. But we've ALWAYS been containers, so not sure why the "text only" thing is there.

On Mon, Nov 1, 2021 at 10:32 AM Ken Dreyer @.***> wrote:

Another thing that's surprising here is that https://errata.devel.redhat.com/advisory/82818 has "rhel8" based containers, and this text-only CDN repo has "rhel-7" in the name.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/red-hat-storage/errata-tool/issues/222#issuecomment-956237870, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXSDPJLZNSQ3C6FV6O2ETUJ2JHZANCNFSM5G7JZ42A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

Nick Boldt (he/him/his)

Principal Software Engineer, RHCSA

Productization Lead :: CodeReady Workspaces

IM: @nickboldt / @nboldt / https://divbyzero.neocities.org https://red.ht/sig TRIED. TESTED. TRUSTED. https://redhat.com/trusted @ @redhatnews https://twitter.com/redhatnews Red Hat https://www.facebook.com/RedHatInc https://www.facebook.com/RedHatInc

“The Only Thing That Is Constant Is Change” - Heraclitus

obriencj commented 2 years ago

Container advisories are not text-only, so it makes sense that the assert would fail.

A text-only advisory and a container advisory push their metadata to an RPM repo in the same way -- as a bare advisory with no RPMs associated. The association of the containers is not based in any way on the CDN repo setting, they have their own mappings. In a way the result of both could be considered a text-only errata, but that setting doesn't exist on the published metadata, only on the data in the errata tool itself.

The rhel-ism and even the arch of a text-only repo is immaterial. I didn't create that particular repo, or I'd have attempted to leave those out of the name. Text-only repos serve exactly one purpose, to exist as a hack to allow advisories to appear on the errata page for products which have no other RHSM presence. CRW offers no RPMs, users have no subscription to browse content on UD. But in order for the advisory to appear at all, there must be one repo associated.

The underlying misunderstanding here is that a container advisory is not text-only.

As to why there's a 404 from the tool, that I cannot answer.

obriencj commented 2 years ago

I think I see the problem. The api for metadataCdnRepos takes a list of repo names, but the example above uses a single string. Try encapsulating that repo name in a list.

edit: yup, I think the 404 is because it's trying to enable a repo named "r" because it's iterating over the enable argument, and rather than being a list of strings it's just a single string.

https://github.com/red-hat-storage/errata-tool/blob/master/errata_tool/erratum.py#L409

nickboldt commented 2 years ago

this...

e = Erratum(errata_id=82818)

e.setState('NEW_FILES')
e.commit()

# set CDN which for some reason is never automatically set when we clone an issue
assert 'docker' in e.content_types
e.metadataCdnRepos(enable=['rhel-7-server-crw-text-only-rpms__x86_64'])

... WORKS!

So the problem is the documentation at https://pypi.org/project/errata-tool/ (see under More Python Examples), which doesn't tell me I have to use enable=['...'] but rather enable='...':

image

obriencj commented 2 years ago

Yeah that doc doesn't align at all with even the docstrings, which clearly state it's a list. Someone should probably submit a pr for the docs to fix that

ktdreyer commented 2 years ago

Great, thank you for clearing this up @obriencj

nickboldt commented 2 years ago

Not sure how this change gets published outside this repo, but here's a PR:

https://github.com/red-hat-storage/errata-tool/pull/223