-
Notifications
You must be signed in to change notification settings - Fork 1
Add list-all-resources endpoint #7
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
base: add-list-resources-endpoint
Are you sure you want to change the base?
Add list-all-resources endpoint #7
Conversation
Harshil2107
commented
Dec 10, 2025
- Add list all resources endpoint
- Add unit tests for the new endpoint
- Update readme with information about the new endpoint
- Add list all resources endpoint - Add unit tests for the new endpoint - Update readme with information about the new endpoint
|
Note: The CI tests will fail in this PR as the ENV variables needed to run the tests will not work when PR is from a fork. Once the changes look good I will merge this PR to the |
Just before I start reviewing, and while I remember: Why do you do things this way? Why are you doing a PR for your changes into a feature branch then a PR for the feature branch into the main? Why not just a PR from your changes into the main? |
For our CI tests we run the functions locally on the action so we need MongoDB seceret and Azure search secrets. The CI tests will only use those secrets when a PR is made from the same repo and not any forks. I do it this way as I can have the feature related changes on my fork, as I donr want to directly push to this repo. |
BobbyRBruce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only big question is, should 'list_all_resources' return literally all gem5-resources with listed compatibility even if many of those are the same resource ID differentiated with different versions? If resource X has 100 versions, all of which are listed as compatibility with a given gem5 version, should it return 100. Is there ever a case where we want this or its desired? Maybe, but it'd be easier if I could get it filtered to just the latest versions of each resource ID and save me the post-processing.
I'm thinking maybe you could have an additional part of the query which turns this filtering on or off. perhaps just a flag, 'latest-compatible-versions', that if set does this server-side
|
|
||
| - Uses prefix matching: a parameter like `25.0.0.1` matches resources with `25.0`, `25.0.0`, or `25.0.0.1` in their `gem5_versions` field | ||
| - Version parameter must have at least `major.minor` format (e.g., `23.0`, not just `23`) | ||
| - Returns all versions of resources that match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it? Is that what we want? Shouldn't this return the more current compatible version of each resource. E.g., if Resource A version 1 and Resource A version 2 both state compatibility with the specified gem5 version then this function would return Resource A version 2 only. Version 1 has been replaced with something, presumably, better. I can't see a case you'd need an older version over the most current.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, currently we return all versions.
I feel it is better to return every version and then post process to the latest one.
I like the idea of adding an optional parameter that will return only the latest compatible version.
By default, we return all compatible version.
| # Build a list of all possible prefixes from the version | ||
| # Starting from major.minor since gem5 versions always have at | ||
| # least one dot (e.g., "25.0", "23.1") | ||
| # e.g., "25.0.0.1" -> ["25.0", "25.0.0", "25.0.0.1"] | ||
| version_parts = gem5_version.split(".") | ||
| prefixes = [] | ||
| for i in range(2, len(version_parts) + 1): | ||
| prefixes.append(".".join(version_parts[:i])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have any real reason to search for compatibility with minor and hotfix values? It kind of makes things more complex. Are there any resources that express compatibility beyond the gem5 major versions? This is more me trying to understand. I assume we do this same procedure elsewhere and it'd be too annoying to change at this stage.
Just to check my assumptions on how this works: What would it mean for a resource of a particular version to have a stated compatibility of say, 'v24.0.1', with a gem5 version of 'v24.0.2'? I take it this gem5 resource would be seen as incompatible? Also, am I correct in assuming when a resource has one stated compatibility at the granularity of the a major release, and another stated compatibility at the granularity of am minor/hotfix release of that same major (.e.g., ['v24.1' 'v24.1.1'], or ['v24.0', 'v24.0.0.1']), then the major/hotfix compatibility is effectively redundant as the major version compatibility assumes compatibility with all releases within that major release (e.g., ['v24.1' 'v24.1.1'] is equal to ['v24.1'] in practice).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We dont currently use the functionality but the get_resources function in gem5 does this for post processing so I am doing this to keep the behavior consistent. I also chose this approach and the gem5_version that is passed by default is always the full version, and I feel it is better to search in the current way than truncating the version to the major release as this approach will limit us in case we want this flexibility in the future.
For your second question, if a particular version to have a stated compatibility of say, 'v24.0.1', then the resource would be incompatible with gem5 version of 'v24.0.2', this would potentially be helpful in case we need to update checkpoint in hot fixes, etc.
Yes, if a resource has one stated compatibility at the granularity of the a major release, and another stated compatibility at the granularity of am minor/hotfix release of that same major (.e.g., ['v24.1' 'v24.1.1'], or ['v24.0', 'v24.0.0.1']), then the major/hotfix compatibility is effectively redundant as the major version compatibility assumes compatibility with all releases within that major release (e.g., ['v24.1' 'v24.1.1'] is equal to ['v24.1'] in practice).
Ok, that resulted in me taking too big a dive into GitHub actions; but yes, it seems you're right. I feel like this limitation should be fixed by Github at some point but yes. Thanks for explaining it to me! |
- Add the `latest-version` param that if set tp true will return only the latest compatible version of the resource. False by default - Add unit tests to test the `latest-version` param - Update readme with information about the `latest-version` param