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

Feature/multiple-binary #45

Closed

Conversation

abdheshnayak
Copy link

Added Support for Binary Name via Query Params

Issue:

When a repository name differs from its binary release name, it could previously not be handled correctly. For example, if the repository name is foobar but the binary name is fb.

Solution:

To address this issue, the parameter ?bin= has been introduced. This allows for the specification of the binary name directly in the query parameters.

Behavior of Changes:

These changes do not affect any previous functionality. The addition of the bin parameter serves as an enhancement, allowing users who need to specify a different binary name to do so. This ensures compatibility and flexibility for various repository and binary naming conventions.

Now it's capable of handling if binary name differs from reponame and also previous state will work as same as it was working.
updated issue with formatting in readme for line query_params -> bin.
@abdheshnayak
Copy link
Author

This PR solves issue #42 .

@jpillora
Copy link
Owner

Thanks though how does this differ from ‘as’?

@abdheshnayak
Copy link
Author

abdheshnayak commented Feb 27, 2024

as handles the binary name in the local system. It doesn't have any relationship with the release name. Downloading the same file doesn't matter what value you are providing through as.

But the above changes will handle which binary should be downloaded and, if one repository consists of more than one binary then we can specify which binary needs to download.

For example in the case of monorepo, one repository can distribute two binaries for example,

Repo Name: foobar

Releasing: fb-client, fb-server

It might be possible to distribute both within a single archive, but users may not need both and we are going to add junk to their system which will be never used in future.

So to provide specific access users can download fb-client by bin=fb-client and similarly fb-server by bin=fb-server.

Next combination that user can specify:
action: download fb-server as fbs
Query: bin=fb-server&as=fbs

action: download fb-client as fbc
Query: bin=fb-client&as=fbc

In the above both scenarios the repository is the same but distributing two binaries client and server through release.

@jpillora
Copy link
Owner

jpillora commented Feb 27, 2024 via email

@@ -110,6 +110,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
User: "",
Program: "",
Release: "",
Selected: r.URL.Query().Get("select"),
Copy link
Author

Choose a reason for hiding this comment

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

As select is reserved keyword in golang, used selected and user can provide select as params.

@abdheshnayak
Copy link
Author

abdheshnayak commented Feb 27, 2024

Changes made accordingly, please check and review.

@abdheshnayak abdheshnayak mentioned this pull request Feb 29, 2024
@thaynes43
Copy link

Will this be merged? I'm trying to grab the omnictl binary from https://github.com/siderolabs/omni/releases/tag/v0.42.3 but right now I don't believe there's a way to specify that over the "omni" binaries. This PR sounds like it would allow me to do that.

I can explore another option if this isn't going in but figured I'd check. Thanks!

@jpillora
Copy link
Owner

You can filter the assets with select https://i.jpillora.com/siderolabs/omni?select=ctl

@jpillora
Copy link
Owner

Implemented something like this in daba077

Sorry couldn't merge this, there was a bit more to it

@jpillora jpillora closed this Oct 10, 2024
@thaynes43
Copy link

You can filter the assets with select https://i.jpillora.com/siderolabs/omni?select=ctl

Thanks! That's exactly what I needed.

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.

3 participants