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

cloudapi: Add /distributions/ to cloudapi #4336

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bcl
Copy link
Contributor

@bcl bcl commented Aug 28, 2024

This pull request includes:

  • adequate testing for the new functionality or fixed issue
  • adequate documentation informing people about the change such as
    • submit a PR for the READMEs listed here
    • submit a PR for the osbuild.org website repository if this PR changed any behavior not covered by the automatically updated READMEs

@bcl bcl force-pushed the main-cloudapi-distros branch from 46ab488 to 5f16411 Compare August 29, 2024 00:21
@bcl bcl marked this pull request as draft August 29, 2024 23:28
@bcl
Copy link
Contributor Author

bcl commented Aug 29, 2024

I'm thinking I want to extend this to return more, so converting to draft for now.

@bcl bcl force-pushed the main-cloudapi-distros branch from 5f16411 to a316c2a Compare August 31, 2024 01:14
@bcl bcl force-pushed the main-cloudapi-distros branch from a316c2a to e8ae6ac Compare September 11, 2024 19:55
@bcl bcl force-pushed the main-cloudapi-distros branch from e8ae6ac to 388076f Compare September 19, 2024 21:11
@bcl bcl marked this pull request as ready for review September 19, 2024 21:16
@bcl bcl force-pushed the main-cloudapi-distros branch 2 times, most recently from bba8569 to 0e142d2 Compare September 26, 2024 17:03
@bcl bcl force-pushed the main-cloudapi-distros branch 2 times, most recently from e798fd1 to 85d1c15 Compare October 15, 2024 21:06
Copy link

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 15, 2024
@bcl bcl force-pushed the main-cloudapi-distros branch from 85d1c15 to d15d3c4 Compare November 15, 2024 18:28
@github-actions github-actions bot removed the Stale label Nov 16, 2024
@bcl bcl requested a review from croissanne November 26, 2024 23:28
@bcl bcl force-pushed the main-cloudapi-distros branch from d15d3c4 to 9274be2 Compare November 26, 2024 23:30
@bcl bcl force-pushed the main-cloudapi-distros branch from 9274be2 to 7372cb0 Compare December 20, 2024 19:42
@bcl bcl requested a review from thozza January 11, 2025 00:15
This adds an actual repository json file for the test-disro.
Without this the repo.ListDistros() function doesn't return any actual
distros. Also includes a test to make sure 1 or more repos are loaded.

Related: RHEL-60133
@bcl bcl force-pushed the main-cloudapi-distros branch from 7372cb0 to 4328ef8 Compare January 11, 2025 00:17
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change.

I don't understand the reason behind returning the full repository configuration for each distro/arch/image_type combination from this endpoint. I think that I'm missing the bigger picture behind this change, especially since the linked Jira task talks only about listing the matrix of supported distro:arch:image_type and nothing about repositories.

In addition to that, The summary of the PR is in contradiction to what this PR does - adding only /distributions/ endpoint and not /distributions/{arch}.

Lastly, I'm not sure if it is a good practice for API to use values as object property names in the returned JSON. IOW, the implemented approach vs. to return a flattened array of objects such as:

[
    {
        "distro": "test-distro-1",
        "arch": "test_arch",
        "image_types": [
            "test_type"
        ]
    },
    ...
]

internal/cloudapi/v2/openapi.v2.yml Outdated Show resolved Hide resolved
@thozza thozza requested a review from ondrejbudai January 13, 2025 05:52
@bcl
Copy link
Contributor Author

bcl commented Jan 13, 2025

I'm trying to remember why I included the actual repo data, and I think it was because we've had some questions about exactly what repos are being used for a build and no way to query that info. It's useful info, and easily available, so I included it.

As for the openapi layout, I couldn't figure out any other way to do it and have variable named keys. The reason for that is that it makes more sense to process the output using keys than it is to have to search through all the results for the distro+arch.

The output from this will be used with composer-cli in order to let the user explore what's available to use for the builds, so more detail is better.

@bcl bcl changed the title cloudapi: Add /distributions/{arch} to cloudapi cloudapi: Add /distributions/ to cloudapi Jan 13, 2025
bcl added 2 commits January 13, 2025 14:37
This adds support for listing all of the supported distributions,
their arches, the image types, and their repository details.

This returns 3 nested json objects. The keys for the first layer are the
distribution names. The 2nd layer's keys are the architectures supported
by that distribution, and the 3rd layer's keys are the image types
supported by that distribution:architecture pair. The value of the 3rd
layer is the repository information.

Resolves: RHEL-60133
@bcl bcl requested a review from kingsleyzissou January 13, 2025 22:38
@bcl bcl force-pushed the main-cloudapi-distros branch from 4328ef8 to 93d0f98 Compare January 13, 2025 22:39
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