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

Container support #1

Open
lanjoni opened this issue Oct 2, 2023 · 6 comments · Fixed by #15
Open

Container support #1

lanjoni opened this issue Oct 2, 2023 · 6 comments · Fixed by #15
Assignees
Labels
enhancement New feature or request

Comments

@lanjoni
Copy link
Owner

lanjoni commented Oct 2, 2023

Okay, we all know that it's not practical for anyone to have all the programming languages installed right? But, what if I had a flag stating that to execute, an alternative command must be executed, that is: a command to be executed in a specific programming language container?

Wow! It would be simply incredible, wouldn't it?

Well, this issue is open for exactly that! Comment here and let's discuss more or open your pull request!

@lanjoni lanjoni added the enhancement New feature or request label Oct 2, 2023
@J0sueTM
Copy link
Contributor

J0sueTM commented Oct 2, 2023

Please assign it to me! We've talked about approaches to this issue, so I could take the lead on it!

@J0sueTM
Copy link
Contributor

J0sueTM commented Oct 7, 2023

Hey @lanjoni. I've done my research and a couple of tests, and I came to a possible solution, which implies on modifications to docker itself.

The library I'll be using is crystal-docker, which doesn't have support for socket based connections e.g: /run/containerd/containerd.sock. Since only TCP based connections are supported for now, a solution I came upon was to modify the startup of the docker daemoni in order to accept TCP connections, which unfortunately implies some security breaches could happen.

I also found this article that explains the benefits of making this change. Besides the security problems, the only backside is that everyone wanting to use the -d flag for docker images/containers, would need to enable remote access to docker.

What do you think about this? I'm open to ideas.

@lanjoni
Copy link
Owner Author

lanjoni commented Oct 7, 2023

Hi @J0sueTM!

Perhaps the solution itself is somewhat simpler than running Crystal per container. Well, the advantage of using Crystal is that we can use an executable with the code already compiled.

The main point may be: we don't need a container to run Crystal but we need containers to run the resolutions of other programming languages. Perhaps it would be more practical not to worry about executing a container in Crystal to communicate with others, after all, the compiled one will be executed on the host and will have to upload containers and execute code from other programming languages, also validating the output of each code executed as it is already doing today.

What do you think about that?

@lanjoni
Copy link
Owner Author

lanjoni commented Oct 15, 2023

Hey, the first PR for this case was merged on #15!

@lanjoni
Copy link
Owner Author

lanjoni commented Oct 15, 2023

Some updates to do:

  • Add support to --verbose flag on --docker
  • Add support to -a (to add arguments instead stdin)

@J0sueTM
Copy link
Contributor

J0sueTM commented Oct 15, 2023

Besides these necessary updates, we've found a couple of things that aren't related first hand to this PR, but still inflicts on it, which are:

  • Refactor module Run. both functions run_test and run_docker have little differences, so an abstraction would be good
  • Fix bug on first run. For some reason, if it's the first time a language is being pulled, it doesn't run the tests right away.
  • Make output from shell commands visible. For example, it would be nice to show the docker pull command visual for the end user.
  • Test some exotic languages that aren't in the list in lang/lang.json. Maybe add those on demand?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants