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

Add a debug build to the Makefile #48

Closed
wants to merge 1 commit into from

Conversation

nickchomey
Copy link

@nickchomey nickchomey commented Sep 28, 2024

Description

Allows for step debugging a connector. More context in this discussion which provides instructions to step debug a connector

ConduitIO/conduit#1724 (reply in thread)

Fixes ConduitIO/conduit-site#176

Quick checks:

  • There is no other pull request for the same update/change.
  • I have written unit tests. (Not applicable, i think)
  • I have made sure that the PR is of reasonable size and can be easily reviewed.

Allows for step debugging a connector. More context in this discussion which provides instructions to step debug a connector 

ConduitIO/conduit#1724 (reply in thread)
@hariso
Copy link
Contributor

hariso commented Sep 30, 2024

@nickchomey Thanks for the contribution! Since you're already on it, how about you also add that script, which runs the "debuggable" build? Basically, that would mean we have the template fully ready for debugging. make build-debug would create the following structure:

.
├── bin
│   └── conduit-connector-foo
├── conduit-connector-foo.sh

That way, you're fully set up for debugging!

@nickchomey
Copy link
Author

Will do!

@nickchomey
Copy link
Author

nickchomey commented Sep 30, 2024

Actually, I dont see how this would work. The template just creates a repo for a new connector, and this PR just adds a debug build to the makefile.

But in order to use that debug build, you have to either copy or symlink it to wherever you have your conduit binary. You would also need to change the port number for each connector. I dont see how the makefile in a connector's repo could or should handle all of that.

What seems to be needed is better (official) documentation on how to set up a development environment for conduit and conduit connectors.

@hariso
Copy link
Contributor

hariso commented Oct 2, 2024

But in order to use that debug build, you have to either copy or symlink it to wherever you have your conduit binary.

Why would you need the Conduit binary, if you want to prepare a debug build for the connector? The idea that I proposed is basically this:
Makefile

.PHONY: build-debug
build-debug:
    go build -gcflags=all="-N -l" -ldflags="-X 'github.com/conduitio/conduit-connector-connectorname.version=${VERSION}'" -o bin/conduit-connector-connectorname cmd/connector/main.go
    # create conduit-connector-connectorname.sh

So it builds bin/conduit-connector-foo and creates conduit-connector-foo.sh. Then you copy the script and the folder bin/ to Conduit's connectors directory, and that's it. Conduit's binary doesn't have to be touched.

As for the port, you're making a good point! We could hash the connector name and get a number from it. That would give you the same port every time you build it and you can more easily debug it.

Btw, what I mentioned isn't a must-have for this PR, just a thought about maybe making it easier to debug a connector.

@nickchomey
Copy link
Author

Good idea with the hash. I'll implement that and make the change you requested

@lovromazgon
Copy link
Member

The core team briefly talked about this PR and we decided to not include it in the template. The standalone connector debugging approach is really only useful for the core team when figuring out a communication issue between Conduit and a standalone connector. On the other hand, for the connector developer, it shouldn't make any difference if they run the connector as a built-in or standalone connector, so given that they are focused on the connector functionality, they might as well take the easy route by debugging it as a built-in connector (as you have noted yourself).

So that's why we decided to instead treat the original issue as a documentation issue, moved it to the conduit-site repo and we'll eventually create a guide for developers on how to debug a connector using the built-in connector approach. I hope that still addresses your concerns!

@nickchomey
Copy link
Author

That makes perfect sense. I agree that testing connectors via a built in connectors is the right way to go, and no one in their right mind would opt for doing it via standalone.

Funny enough, I just remembered this issue/PR last night and intended to look at finishing it today. Good timing that you closed it now!

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.

doc: Step Debugging a connector
3 participants