[OTA-2089] fall back to local source for plc_lookup - #45
Conversation
65e5b71 to
ee6eb76
Compare
caf8fcf to
c6b66eb
Compare
c6b66eb to
a8fabb4
Compare
jrangelramos
left a comment
There was a problem hiding this comment.
Just a few nits. Also perhaps data/product.json should be back to the repo (and later addressed/updated via PR with github action or something similar)
1471a6f to
384507c
Compare
jrangelramos
left a comment
There was a problem hiding this comment.
Hi Ankita! Thanks for the fallback code. I've tested with many combinations using both live and fallback data and it looks it mirrors the search logic properly. Nice job! Just a note about connectivity check, it looks redundant and can be simplified.
| API_BASE = "https://access.redhat.com/product-life-cycles/api/v2/products" | ||
| PRODUCTS_PATH = os.path.join(os.path.dirname(__file__), "data", "products.json") | ||
|
|
||
| def check_connectivity(url, timeout=5): |
There was a problem hiding this comment.
I think this could be simplified. The same checks are already performed on the api_search so maybe you can just call the fallback function in the exception handling of the api_search. Having this check also makes the live api to be called twice in a happy path where API is available. So perhaps, consider removing it.
There was a problem hiding this comment.
Good catch, moved the fallback logic to when the api is unreachable
|
Hello @ankitathomas the test |
cc4a71c to
abb7269
Compare
|
/lgtm |
|
/verified by @jrangelramos Ran locally a bunch of lookup test and compared results with live api. |
|
@jrangelramos: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
abb7269 to
cb21827
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ankitathomas The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/lgtm |
| import urllib.request | ||
|
|
||
| API_BASE = "https://access.redhat.com/product-life-cycles/api/v2/products" | ||
| PRODUCTS_PATH = os.path.join(os.path.dirname(__file__), "data", "products.json") |
There was a problem hiding this comment.
This file isn't committed so wouldn't be in the final image built from this repo. Is there any plan for this to be committed and/or mounted into wherever this will be run?
There was a problem hiding this comment.
yes, the file will be updated by a periodic job calling cluster-update/product-lifecycle/scripts/plc_snapshot.sh on changes to the contents served by the product lifecycle api. The file is not present yet due to some concerns about the 5.0 lifecycle information's availability via the api.
|
Hello @ankitathomas Apparently there is still some logic mismatch when search criteria contains comma. with live api $ python3 plc_lookup.py products "mcp,openshift"
{
"error": "no products found",
"query": "mcp,openshift"
}
$ python3 plc_lookup.py products "compliance,operator"
{
"error": "no products found",
"query": "compliance,operator"
}with fake url -> fallback to products.json $ python3 plc_lookup.py products "mcp,openshift" | wc -l
15061
$ python3 plc_lookup.py products "compliance,operator"
Traceback (most recent call last):
File "/home/jeramos/pixaa/test/test-ocp5/agentic-skills/cluster-update/product-lifecycle/scripts/plc_lookup.py", line 266, in <module>
sys.exit(main())
~~~~^^
File "/home/jeramos/pixaa/test/test-ocp5/agentic-skills/cluster-update/product-lifecycle/scripts/plc_lookup.py", line 262, in main
return handlers[parsed.command](parsed, output=output)
~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jeramos/pixaa/test/test-ocp5/agentic-skills/cluster-update/product-lifecycle/scripts/plc_lookup.py", line 138, in cmd_products
results.append(format_product_version(p, v, target_ocp=target_ocp))
~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jeramos/pixaa/test/test-ocp5/agentic-skills/cluster-update/product-lifecycle/scripts/plc_lookup.py", line 107, in format_product_version
"version": version["name"],
~~~~~~~^^^^^^^^
KeyError: 'name' |
Signed-off-by: Ankita Thomas <ankithom@redhat.com>
Signed-off-by: Ankita Thomas <ankithom@redhat.com>
4959560 to
8a9223b
Compare
Good catch! Turns out two entries in the product lifecycle api are being shipped without a version. That seems like a mistake, but I've added some checks so the script does not panic. |
|
@ankitathomas: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
/verified by @jrangelramos |
|
@jrangelramos: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Include product lifecycle data as a file in agentic-skills image and have plc_lookup.py use the local file if the products lifecycle API is unreachable, such as in a disconnected environment.