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

Reduce resolver dependencies on operator-registry and operator-lifecycle-manager. #2337

Conversation

benluddy
Copy link
Contributor

This is a continuation of the ongoing work to allow the runtime dependency resolution features of catalog-operator to be reusable outside of OLM.

Signed-off-by: Ben Luddy <bluddy@redhat.com>
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 26, 2021
@openshift-ci
Copy link

openshift-ci bot commented Aug 26, 2021

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2021
@benluddy benluddy changed the title Reduce resolver dependencies on operator-registry and operator-lifecycle-manager imports. Reduce resolver dependencies on operator-registry and operator-lifecycle-manager. Aug 26, 2021
@benluddy
Copy link
Contributor Author

#2091

The most significant change is the introduction of a new Source
interface, which represents a single source of cache entries. In the
existing implementation, the cache was hardcoded to fetch content from
the operator-registry gRPC API on misses. This was a barrier to
off-cluster tools and other applications that should be able to
perform resolution without running a registry server.

A number of test fixtures were affected (for the better) because it's
now possible to directly configure the real Cache implementation to
retrieve test content without using of test doubles.

Signed-off-by: Ben Luddy <bluddy@redhat.com>
@benluddy benluddy marked this pull request as ready for review August 27, 2021 13:41
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 27, 2021
Copy link
Member

@dinhxuanvu dinhxuanvu left a comment

Choose a reason for hiding this comment

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

One tiny nit but it doesn't really matter.
/lgtm

s.snapshots[i], s.snapshots[j] = s.snapshots[j], s.snapshots[i]
}

func (s *CatalogSnapshot) Find(p ...OperatorPredicate) []*Operator {
func (s *snapshotHeader) Find(p ...OperatorPredicate) []*Operator {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Can we move all of types to the head of the file? It is difficult to review and understand the code when you use a type at the top of the file and the type itself is declared somewhere at the bottom of the file.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 1, 2021
@openshift-ci
Copy link

openshift-ci bot commented Sep 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benluddy, dinhxuanvu

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 [benluddy,dinhxuanvu]

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants