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

repository api(s) #23

Merged
merged 4 commits into from
May 11, 2023
Merged

repository api(s) #23

merged 4 commits into from
May 11, 2023

Conversation

henderiw
Copy link
Contributor

This is the repository api to creates repos

@nephio-prow nephio-prow bot requested review from s3wong and tliron May 10, 2023 07:48
Copy link
Member

Choose a reason for hiding this comment

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

Did we just miss a generate so this was out of date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was out of date it seems. We might need to integrate this as a prow job.

Copy link
Member

Choose a reason for hiding this comment

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

So, is this to be used to provision repositories, or is this used to specify a package's requirements for a repository? I prefer to decouple those things. I also think we may not need the latter (or, rather, I may want to handle it as a generic package dependency instead, which we haven't figure out yet).

If it's for provisioning, I would think it belongs in an API group that goes with the repo provisioning controller, rather than in the general "infra" group. Just like we have talked about providing different packages for different cluster providers, we could provide different packages for different repo providers. This would be instead of abstracting the repo concept and having different implementors.

All that said, I am pretty much OK with just getting something working in R1; we can debate the details later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a bit if a mixed scenario right now and the controller could be implemented as a specialised or a real controller. The main difference I see is the feedback loop is different. Right now we decided to work as a controller so it will be used to provision a repo.
I agree we should look at this after R1 and maybe restructure things based on the patterns we see fit best for these scenario's.

@johnbelamaric
Copy link
Member

/approve

@nephio-prow nephio-prow bot added the approved label May 10, 2023
@henderiw
Copy link
Contributor Author

/approve

@nephio-prow
Copy link
Contributor

nephio-prow bot commented May 11, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: henderiw, johnbelamaric

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [henderiw,johnbelamaric]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gvbalaji
Copy link

/lgtm

@nephio-prow nephio-prow bot added the lgtm label May 11, 2023
@nephio-prow nephio-prow bot merged commit 3316d8b into nephio-project:main May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants