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

✨ adding reflection client to determine what provider provides #586

Merged

Conversation

shawn-hurley
Copy link
Contributor

@shawn-hurley shawn-hurley commented Apr 22, 2024

Fixes #573
DRAFT FOR NOW

initial implementation some TODO's left want to see what CI gives me

@shawn-hurley shawn-hurley force-pushed the feature/add-location-grpc-services branch 4 times, most recently from 8d039d3 to 9a92350 Compare April 23, 2024 01:14
@shawn-hurley
Copy link
Contributor Author

This is breaking because we are not updating the external providers to use the updated code in analyzer-lsp during the build here.

going to try and fix this

@shawn-hurley shawn-hurley force-pushed the feature/add-location-grpc-services branch 2 times, most recently from 3890234 to 1d49ab2 Compare April 23, 2024 14:07
@@ -1,15 +1,18 @@
# Build the manager binary
FROM golang:1.21 as builder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to downgrade to 1.20

@shawn-hurley shawn-hurley force-pushed the feature/add-location-grpc-services branch from 671b52f to b823117 Compare April 23, 2024 19:00
@shawn-hurley
Copy link
Contributor Author

Hey @pranavgaikwad @eemcmullan I am really sorry this feature PR turned into a catch all PR fixing CI, making all the docker files build with same go as well as the feature.

if you really want, I can make more commits to make it easier to review but let me know what you think

Copy link
Contributor

@pranavgaikwad pranavgaikwad left a comment

Choose a reason for hiding this comment

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

Looking good overall.

shawn-hurley and others added 8 commits April 23, 2024 17:01
Signed-off-by: Shawn Hurley <[email protected]>
Signed-off-by: Shawn Hurley <[email protected]>
Signed-off-by: Shawn Hurley <[email protected]>
Signed-off-by: Shawn Hurley <[email protected]>
Signed-off-by: Pranav Gaikwad <[email protected]>
@shawn-hurley shawn-hurley force-pushed the feature/add-location-grpc-services branch 3 times, most recently from ee9f682 to 1b30f6a Compare April 24, 2024 16:20
Copy link
Contributor

@pranavgaikwad pranavgaikwad left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you very much for fixing everything

.github/workflows/demo-testing.yml Outdated Show resolved Hide resolved
@@ -25,6 +28,6 @@ RUN npm install -g typescript-language-server typescript
RUN go install golang.org/x/tools/gopls@latest

COPY --from=go-builder /generic-external-provider/generic-external-provider /usr/local/bin/generic-external-provider
COPY --from=go-dep-provider /usr/local/bin/go-dependency-provider /usr/local/bin/go-dependency-provider
COPY --from=go-dep-provider /usr/local/bin/golang-dependency-provider /usr/local/bin/golang-dependency-provider
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: will need to update this in kantra now. I can do so. I just don't want to forget

@@ -1,13 +1,15 @@
FROM golang:1.20 as go-builder

copy / /analyzer-lsp
Copy link
Contributor

Choose a reason for hiding this comment

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

typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, I think that is moving the / context into /analyzer-lsp in the image.

}
}

func checkServicesRunning(refClt *reflectClient.Client, log logr.Logger) ([]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

+100

Signed-off-by: Shawn Hurley <[email protected]>
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.

[BUG] codeSnips for java.dependency rules do not work with external java provider
3 participants