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

Rewrite TeleIRC from NodeJS to Go #163

Closed
jwflory opened this issue Aug 31, 2019 · 17 comments
Closed

Rewrite TeleIRC from NodeJS to Go #163

jwflory opened this issue Aug 31, 2019 · 17 comments

Comments

@jwflory
Copy link
Member

@jwflory jwflory commented Aug 31, 2019

Summary

Rewrite TeleIRC from NodeJS to Go

Background

Recently we discussed a semester mini-project of rewriting/porting TeleIRC to Go for the following reasons:

  • Primary goals:
    • Distinguish our project from the other NodeJS teleirc bot
    • Make a better appeal to the k8s/OpenShift community with a Go app
  • Secondary goals:
    • @Tjzabel and @jwflory are looking for opportunities to work more with Go for learning
    • @Zedjones has more background / experience in Go and was interested in idea of porting
    • Better-performing language choice for networking-heavy use of our bot
    • Intention to make the code more maintainable and easily testable

For @Tjzabel and I on the core maintaining team, we are both looking for opportunities to learn more about programming in Go. It would be a learning opportunity to pick up a language used often in the spaces we work in. We also have support from @Zedjones, who has used Go for a while longer than us. This should help us start on the right page.

My gamble is whether porting over would actually help us find more downstream contributors to work on the project and help with development. One real-world use case is improving the sustainability of TeleIRC for the Fedora Project, where a NodeJS app is not something often deployed by the infrastructure community, thus distancing itself from the community of people who volunteer to maintain its infrastructure.

Details

A rewrite is a non-trivial investment of time and resources. Before starting, we should map out the pieces of the existing TeleIRC. How does it all fit together? What pieces can be segmented off together? We should also take some time to learn the structure of a good Go project. Any best practices for structuring our project? Can we set up the project in a specific way now to make future packaging easier?

A full developer meeting should be devoted to planning this change and dividing it into smaller pieces if this will be selected as the semester deliverable.

Outcome

  • Distinguish our project from the other NodeJS teleirc bot
  • Make a better appeal to the k8s/OpenShift community with a Go app
  • Better-performing language choice for networking-heavy use of our bot
@jwflory jwflory added this to Backlog in TeleIRC development via automation Aug 31, 2019
@Tjzabel Tjzabel moved this from Backlog to In progress in TeleIRC development Aug 31, 2019
@xforever1313

This comment has been minimized.

Copy link
Contributor

@xforever1313 xforever1313 commented Aug 31, 2019

Go is statically typed and compiled, which to me makes it superior than node, at least on paper. I personally don't know much about Go, so pardon my ignorance here.

Some things to consider:

  • Does Go have an event handler system like Node's EventEmitter or C# Events? We use these heavily in the Node implementation. Events are usually more efficient than polling.
  • Our Node implementation uses an IRC library, Telegram library, Imgur library, and escapes HTML. Does GO have equivalent libraries we can use?
  • Does it have a decent Unit Test Library and Mocking Library?
  • Does it have a concept of async/await/promises? For example, when uploading an image to imgur, we probably don't a thread to hang while uploading the image. Async/Await allows the thread to unblock and listen to more messages while the image is being uploaded in a background thread.

Some design considerations:

  • Do we want to keep our current JSON configuration file (in which case we need to find a JSON library) or blow it up and do something different
  • We luckily have a relatively OOP design of TeleIRC now, so porting those classes over to GO should not be a problem. Honestly, configuration if the only portion that could be greatly improved in this aspect.
@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 1, 2019

Does Go have an EventEmitter equivalent?

Go does have a very good EventEmitter system, in the form of channels

Does Go have appropriate library equivalents?

  • IRC Libraries
  • Telegram library
  • Imgur Library
    • I cannot yet find a good option written in Go that handles our use-case
      • Given the relative simplicity of the Imgur API, we may potentially have to write our own. Otherwise, we can continue looking for library options here, or even consider other image hosting options.
  • HTML Escape

Unit Test Libraries

  • Standard testing library covers unit tests
  • GoMock is the official Mock framework

Await/Async?

  • Go excels in asynchronous/concurrency needs via goroutines.
@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 2, 2019

As a side note, I've been experimenting in my head with how a port to Go would work.

I believe it would take a solid 3 folks with TeleIRC as their main/only project in order to finish this by the end of this semester (if that's our goal).

Everything exists (except maybe the Imgur library) for this port to go smoothly. The main factor for this move is time. The main questions really are:

  1. Will porting TeleIRC to Go bring in more outside contributors?
  2. Will porting to Go allow us to more effectively maintain TeleIRC in the longterm?
  3. Will porting to Go solve any problems that we currently have with NodeJS?
  4. Is the time this will take worth the effort?

I personally think the answer to these questions is yes. Go seems perfect for our use case, and excels in this space. Having this as a Go project will also finally separate us from the other TeleIRC project.

Question: As we are entering the beginning of the semester, and have started community outreach to gather more contributors, how long do we want to timebox this discussion for before we move forward? I effectively see this issue as a blocker for all other issues at this time.

@jwflory

This comment has been minimized.

Copy link
Member Author

@jwflory jwflory commented Sep 3, 2019

Question: As we are entering the beginning of the semester, and have started community outreach to gather more contributors, how long do we want to timebox this discussion for before we move forward? I effectively see this issue as a blocker for all other issues at this time.

I believe this decision should be final by the first development meeting.

Involving new folks is easier if we have a (loose) roadmap of a major project goal/target to work towards. It can be overwhelming to volunteer for a new project and get asked, "What do you think we should start?" Instead, I prefer to show up with ideas of different directions to steer this semester and then see how to engage new folks to join based on their interests and areas of preference.

I think a Go rewrite is a potential semester goal.

@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 3, 2019

@jwflory sounds good. Is there currently a relative time when we want to have our first developer meeting?

As a side note, I have confirmed a Go bot works from both IRC and Telegram side, given the libraries I have linked above.

(Edit) Another great reason to consider a port to Go is the extreme simplicity of third party libraries. Our current NodeJS implementation pulls in quite a few external libs, while most of these Go libraries are simply written it Go's standard library. This will greatly increase security.

@Zedjones

This comment has been minimized.

Copy link
Member

@Zedjones Zedjones commented Sep 6, 2019

Having used Go for quite a few projects, the only real sticking point that I've run into is mocking/unit tests. GoMock does work well, when it's possible to use it. However, it only works on interfaces. So, if you are using a library that provides a struct and want to mock it, you have to wrap it inside of an interface and then mock that interface. This requires changes to the source code, which isn't the best. However, functional/integration tests are pretty well documented. As far as a standard structure for Golang projects, I think this repo is a pretty good example.

@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 6, 2019

@Zedjones thanks for the input!

That's interesting about the Mock library. I wonder why it would function like that.

@Zedjones

This comment has been minimized.

Copy link
Member

@Zedjones Zedjones commented Sep 6, 2019

@Zedjones thanks for the input!

That's interesting about the Mock library. I wonder why it would function like that.

Since Go isn't object-oriented and as such there's no way to subclass a struct, if you're accepting a struct, there's no way to fake a mock as an instance of that struct. The mocking library would either have to modify your source code or the module's source code either way, so instead they just opted to support interfaces.

@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 7, 2019

Ah, makes sense.

@Zedjones

This comment has been minimized.

Copy link
Member

@Zedjones Zedjones commented Sep 8, 2019

go-imgur might work for our imgur needs. We'd need to download the image from Telegram into memory first and then pass that byte array to the function provided by go-imgur, but it's just one more step than what we do now (passing the URL directly to the imgur library in Node).

@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 8, 2019

@Zedjones thanks for looking into that!

@Tjzabel Tjzabel added this to Backlog in Port to Go via automation Sep 13, 2019
@Tjzabel Tjzabel removed this from In progress in TeleIRC development Sep 13, 2019
@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 13, 2019

Hmm... The next challenge I'm working through, is figuring out how the project board should be laid out as we do this move to Go. Do we want to have a separate project board for the Go port, or do we want to move everything we currently have in TeleIRC Development to the backlog, and continue from there?

@Tjzabel Tjzabel moved this from Backlog to In Progress in Port to Go Sep 14, 2019
@jwflory jwflory removed this from In Progress in Port to Go Sep 14, 2019
@Tjzabel Tjzabel added this to Backlog in TeleIRC development via automation Sep 15, 2019
@Tjzabel Tjzabel moved this from Backlog to In progress in TeleIRC development Sep 15, 2019
@Tjzabel Tjzabel added this to the v2.0 milestone Sep 15, 2019
@Tjzabel Tjzabel pinned this issue Sep 15, 2019
@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Sep 21, 2019

@jwflory what do we want to do with this issue? Do we want to keep this open forever, as a pinned issue until we have completed development?

@jwflory jwflory removed this from In progress in TeleIRC development Oct 22, 2019
@jwflory

This comment has been minimized.

Copy link
Member Author

@jwflory jwflory commented Nov 11, 2019

@Tjzabel I think it is okay to keep this issue open as a pinned issue. In the future, we could work towards putting meeting minutes / notes in this issue to track our progress over time. This doesn't need to be on our project board though since it is persistent over time.

@jwflory

This comment has been minimized.

Copy link
Member Author

@jwflory jwflory commented Mar 12, 2020

Hey @Tjzabel @Zedjones @kennedy @10eMyrT, how do we feel about cutting a v1.0.0-pre1 release since basic bridging is functional? We are not at feature-parity with v1.x.x yet, but here's my thinking for cutting a release:

  • Stop maintaining two branches of code (master and go-port)
  • Default git branch becomes Go branch, which also makes our project publicly visible as Go and not Node (so we don't accidentally bring in contributors for v1.x.x)
  • Begin experimenting with running #rit-lug-teleirc with latest master
  • Might attract a few more people to help test out the new Go code and give us feedback

Not sure if there is a better place to start this conversation but I figured this issue is as good as anywhere. Thoughts?

@jwflory jwflory removed this from the v2.0.0 milestone Mar 17, 2020
@Tjzabel

This comment has been minimized.

Copy link
Member

@Tjzabel Tjzabel commented Mar 29, 2020

@jwflory now that we've merged the go-port branch into master, what is the closing criteria for this issue? Do we want to keep this open until we've officially hit v2.0?

@jwflory

This comment has been minimized.

Copy link
Member Author

@jwflory jwflory commented Mar 30, 2020

@Tjzabel Good point. I think the usefulness of this issue has been served. We're obviously not "done" with the go-port, but between the v2.0.0 milestone and the individual issues we are using for more detailed discussions, I think we can close this issue out because we are past the point of discussing whether we want to do a Go port or not. We are at pre-releases now, we are all in 😄

So I'm going to close this as complete. 🎬 Let's keep tracking progress in the v2.0.0 milestone and other issues that we're working through on the sprint project board.

@jwflory jwflory closed this Mar 30, 2020
@jwflory jwflory unpinned this issue Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.