-
Notifications
You must be signed in to change notification settings - Fork 702
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
Add initial echo kubeapps-apis service with cobra-generated command. #2784
Conversation
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.
Awesome! Delighted to see this very initial step of this Kubeapps API service! Thank you!
cmd/kubeapps-apis/Makefile
Outdated
# Copyright 2021 VMware. All Rights Reserved. | ||
# SPDX-License-Identifier: Apache-2.0 |
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.
In fact, I'll update the copyright back to match the rest of the project for now until we've sorted that out.S
So, according to that, you will update it back, no? I agree, let's hold any change until we figure out how a uniform way to tackle this problem.
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.
Oh - I did already (copyright to VMware, not Kubeapps contributors). Or you mean we should also not use the spdx identifier? OK, I'll switch that back to the normal apache summary.
@@ -0,0 +1,40 @@ | |||
# Experimental Kubeapps APIs service | |||
|
|||
This is an experimental service that we are not yet including in our released Kubeapps chart, providing an extensible protobuf-based API service enabling Kubeapps (or other packaging UIs) to interact with various Kubernetes packaging formats in a consistent way. |
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.
I'd also mention the multiplexed HTTP API, no?
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, I'll do that when I add it in a subsequent PR.
// This file is not intended to be compiled. Because some of these imports are | ||
// not actual go packages, we use a build constraint at the top of this file to | ||
// prevent tools from inspecting the imports. |
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.
Interesting, thanks for the info here!
@@ -0,0 +1,40 @@ | |||
# Experimental Kubeapps APIs service |
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.
So this component isn't yet behind a feature flag, no? I guess the idea is to first have the new chart, and then iterate over it adding new features.
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.
Right, it isn't referenced or used anywhere in our chart or the rest of the code yet. When I add support for deploying it, I'll have that behind a feature flag.
Description of the change
Adds the initial file layout with a single cobra command that starts an echo service for the experimental kubeapps-apis service.
Benefits
Committing this mostly-generated cobra command, together with some initial decisions about licensing and naming, so that following PRs can be focused on actual code.
Possible drawbacks
We may need to find out more about a contributor agreement before adding Kubeapps contributors. In fact, I'll update the copyright back to match the rest of the project for now until we've sorted that out.
Applicable issues
Additional information
I'll keep the README.md updated as each PR is proposed. Regarding the name, I've included kubeapps in the command name itself (ie.
kubeapps-apis
rather thanapis-service
) in case it is re-used externally at some point (as we're writing the service so that it potentially can be helpful for other UIs too).