[Challenge][Http-Server][Language support][Clojure] Add clojure support to http-server-challenge #112
Conversation
WalkthroughThis update standardizes and extends the build and execution infrastructure for multiple Clojure projects. New shell scripts for compiling, running, and local execution have been introduced across several directories. Build scripts, dependency descriptors, and configuration files have been added or updated to define the project’s structure, manage dependencies, and standardize Git and Docker settings. In addition, the main entry points now include a basic server socket setup with error handling. These changes affect starter templates, CodeCrafters solutions, and compiled starter projects. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CS as Compile Script
participant BS as Build Script
participant RS as Run Script
participant App as Java App (-main)
participant Client as Client
Dev->>CS: Execute compile.sh
CS->>BS: Trigger build (clean, jar, uber)
BS-->>CS: Build complete
Dev->>RS: Execute run.sh or your_program.sh
RS->>App: Run standalone JAR
App->>App: Initialize server socket (port 4221)
Client->>App: Attempt connection
App-->>Client: Accept connection and log event
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (18)
starter_templates/clojure/config.yml (1)
1-3: Configuration Attributes Added for ClojureThe new attributes clearly define the required executable and the main user-editable file for the Clojure starter template. These entries will help ensure that the build process and user modifications are correctly guided.
Suggestion: For enhanced clarity and to avoid any potential YAML parsing issues with special characters, consider quoting the version string. For example:
required_executable: "clj (1.12.0)"starter_templates/clojure/code/src/main.clj (2)
4-7: Follow Clojure style guidelines for parentheses.The closing parenthesis should be on the same line as the last expression, not on a new line.
(defn -main [& args] ;; You can use print statements as follows for debugging, they'll be visible when running tests. - (println "Logs from your program will appear here!") - ) + (println "Logs from your program will appear here!"))Also applies to: 16-16
8-15: Improve socket resource management.While the basic socket setup is good, consider these improvements for the commented code:
- Use
with-opento ensure socket cleanup- Set accept timeout to prevent indefinite blocking
- Add proper resource cleanup in a finally block
;; Uncomment this block to pass the first stage -;; (try -;; (let [server-socket (java.net.ServerSocket. 4221)] -;; (doto server-socket -;; (.setReuseAddress true) -;; (.accept)) -;; (println "accepted new connection")) -;; (catch java.io.IOException e -;; (println (str "IOException: " (.getMessage e))))) +;; (try +;; (with-open [server-socket (doto (java.net.ServerSocket. 4221) +;; (.setReuseAddress true) +;; (.setSoTimeout 10000))] +;; (println "Waiting for connection...") +;; (.accept server-socket) +;; (println "Accepted new connection")) +;; (catch java.net.SocketTimeoutException e +;; (println "Timeout waiting for connection")) +;; (catch java.io.IOException e +;; (println (str "IOException: " (.getMessage e))) +;; (finally +;; (println "Server stopped"))))solutions/clojure/01-at4/code/your_program.sh (1)
20-24: Execution Block & Hardcoded JAR Path.The execution block uses
execto run the Java application by invoking the standalone JAR, which is efficient as it replaces the shell process. However, the JAR file path is hardcoded as/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar. Please verify that this path reliably points to the expected JAR file in all targeted environments. If there’s any chance that the path might vary (e.g., due to different deployment setups or runtime configurations), consider parameterizing this value through an environment variable or configuration file.solutions/clojure/01-at4/explanation.md (1)
5-15: Code block: Consider resource management improvements.
The provided Clojure code block for setting up a server socket is clear and demonstrates basic error handling forjava.io.IOException. As a future enhancement, consider using thewith-openmacro to ensure that the server socket is automatically closed after use. This can help prevent resource leaks when moving from the initial stage to a more robust implementation. For example:- (let [server-socket (java.net.ServerSocket. 4221)] - (doto server-socket - (.setReuseAddress true) - (.accept)) - (println "accepted new connection")) + (with-open [server-socket (java.net.ServerSocket. 4221)] + (.setReuseAddress server-socket true) + (let [client-socket (.accept server-socket)] + (println "accepted new connection")))Additionally, remember that this block is meant to be uncommented to progress through the challenge, so keep the instructional context clear.
solutions/clojure/01-at4/code/README.md (2)
6-8: Minor Grammar Adjustment Recommended.
On line 7, consider changing “a HTTP/1.1 server” to “an HTTP/1.1 server” for grammatical correctness.Suggested Diff:
- In this challenge, you'll build a HTTP/1.1 server + In this challenge, you'll build an HTTP/1.1 server🧰 Tools
🪛 LanguageTool
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...(EN_A_VS_AN)
29-37: Consider Sequential List Numbering.
The "Stage 2 & beyond" section uses repeated "1." for each list item. While Markdown auto-numbering works fine, using sequential numbers (e.g., 1., 2., 3.) might enhance clarity for readers.compiled_starters/clojure/src/main.clj (2)
4-7: Consider enhancing debug logging.The debug logging is helpful, but consider adding more context such as server port and startup time.
-(println "Logs from your program will appear here!") +(println (str "Starting HTTP server on port 4221 at " (java.time.LocalDateTime/now)))
8-16: Consider usingwith-openfor resource management.The commented socket code should use
with-opento ensure proper resource cleanup.-;; (try -;; (let [server-socket (java.net.ServerSocket. 4221)] -;; (doto server-socket -;; (.setReuseAddress true) -;; (.accept)) -;; (println "accepted new connection")) -;; (catch java.io.IOException e -;; (println (str "IOException: " (.getMessage e))))) +;; (try +;; (with-open [server-socket (doto (java.net.ServerSocket. 4221) +;; (.setReuseAddress true))] +;; (println "Server started, waiting for connections...") +;; (with-open [client-socket (.accept server-socket)] +;; (println "Accepted new connection from" (.getRemoteSocketAddress client-socket)))) +;; (catch java.io.IOException e +;; (println (str "IOException: " (.getMessage e)))))compiled_starters/clojure/build.clj (3)
5-10: Consider adding validation for project configuration.The project configuration reads from
deps.ednbut doesn't validate the structure. Consider adding validation to ensure required fields are present.-(def project (-> (edn/read-string (slurp "deps.edn")) - :aliases :neil :project)) -(def lib (or (:name project) 'my/lib1)) - -(def version (or (:version project) - "0.0.1")) +(def project (try + (-> (edn/read-string (slurp "deps.edn")) + :aliases :neil :project) + (catch Exception e + (println "Warning: Failed to read project configuration:" (.getMessage e)) + {}))) + +(def lib (or (:name project) + (do + (println "Warning: Project name not found in deps.edn, using default") + 'my/lib1))) + +(def version (or (:version project) + (do + (println "Warning: Version not found in deps.edn, using default") + "0.0.1")))
11-14: Consider using a more flexible path configuration.The paths for JAR files are hardcoded. Consider making them configurable through environment variables or command-line arguments.
-(def uber-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar") -(def jar-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure.jar") +(def build-dir (or (System/getenv "BUILD_DIR") + "/tmp/codecrafters-build-http-server-clojure")) + +(def uber-file (str build-dir "/codecrafters-http-server-clojure-standalone.jar")) +(def jar-file (str build-dir "/codecrafters-http-server-clojure.jar"))
38-48: Consider adding progress logging to the uber task.The uber task performs multiple steps but doesn't provide feedback. Consider adding progress logging.
(defn uber [_] + (println "Cleaning target directory...") (clean nil) + (println "Copying source files...") (b/copy-dir {:src-dirs ["src" "resources"] :target-dir class-dir}) + (println "Compiling Clojure sources...") (b/compile-clj {:basis basis :src-dirs ["src"] :class-dir class-dir}) + (println "Creating uber JAR...") (b/uber {:class-dir class-dir :uber-file uber-file :basis basis - :main 'main})) + :main 'main}) + (println "Build completed successfully:" uber-file))starter_templates/clojure/code/build.clj (2)
13-14: Consider making output paths configurable.The JAR file paths are hardcoded under
/tmp. Consider making these paths configurable through environment variables or configuration files to support different environments and improve flexibility.-(def uber-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar") -(def jar-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure.jar") +(def uber-file (or (System/getenv "UBER_JAR_PATH") + "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar")) +(def jar-file (or (System/getenv "JAR_PATH") + "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure.jar"))
16-57: Add docstrings to build functions.Consider adding docstrings to the build functions to improve code documentation and maintainability.
-(defn clean [_] +;; Cleans the target directory by deleting it +(defn clean + "Deletes the target directory to ensure a fresh build." + [_] (b/delete {:path "target"})) -(defn jar [_] +;; Creates a JAR file with compiled classes and resources +(defn jar + "Creates a JAR file by writing POM, copying sources and resources." + [_] ...) -(defn install [_] +;; Installs the JAR to local repository +(defn install + "Installs the generated JAR file into the local repository." + [_] ...) -(defn uber [_] +;; Creates an uber JAR with all dependencies +(defn uber + "Creates an uber JAR containing all dependencies and compiled sources." + [_] ...) -(defn deploy [opts] +;; Deploys the JAR to a remote repository +(defn deploy + "Deploys the JAR file to a remote repository with given options." + [opts] ...)starter_templates/clojure/code/.codecrafters/run.sh (1)
1-11: LGTM with a suggestion for improvement.The script follows good practices:
- Uses proper shebang
- Enables error handling with
set -e- Correctly passes arguments with
"$@"Consider making the JAR path configurable through an environment variable to match the suggested changes in
build.clj.-exec java -jar /tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar "$@" +exec java -jar "${UBER_JAR_PATH:-/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar}" "$@"compiled_starters/clojure/README.md (1)
6-8: Grammatical nitpick: Use "an" instead of "a" before HTTP.
In the sentence "In this challenge, you'll build a HTTP/1.1 server," consider changing it to "build an HTTP/1.1 server" for grammatical correctness.- build a HTTP/1.1 server + build an HTTP/1.1 server🧰 Tools
🪛 LanguageTool
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...(EN_A_VS_AN)
solutions/clojure/01-at4/diff/src/main.clj.diff (1)
5-24: LGTM! Basic server socket setup looks good.The implementation correctly sets up a server socket on port 4221 with proper error handling.
Consider handling multiple connections.
The current implementation only accepts a single connection and then exits. Consider wrapping the
.acceptcall in a loop to handle multiple connections.(defn -main [& args] (try (let [server-socket (java.net.ServerSocket. 4221)] (doto server-socket - (.setReuseAddress true) - (.accept)) - (println "accepted new connection")) + (.setReuseAddress true)) + (while true + (.accept server-socket) + (println "accepted new connection"))) (catch java.io.IOException e (println (str "IOException: " (.getMessage e))))) )solutions/clojure/01-at4/code/build.clj (1)
5-10: Add validation for project metadata.The code provides fallback values for missing project metadata but doesn't validate the values when present.
Consider adding validation:
(def project (-> (edn/read-string (slurp "deps.edn")) :aliases :neil :project)) +(when (and project + (or (not (symbol? (:name project))) + (not (string? (:version project))))) + (throw (ex-info "Invalid project metadata" + {:name (:name project) + :version (:version project)}))) (def lib (or (:name project) 'my/lib1)) (def version (or (:version project) "0.0.1"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
compiled_starters/clojure/.codecrafters/compile.sh(1 hunks)compiled_starters/clojure/.codecrafters/run.sh(1 hunks)compiled_starters/clojure/.gitattributes(1 hunks)compiled_starters/clojure/.gitignore(1 hunks)compiled_starters/clojure/README.md(1 hunks)compiled_starters/clojure/build.clj(1 hunks)compiled_starters/clojure/codecrafters.yml(1 hunks)compiled_starters/clojure/deps.edn(1 hunks)compiled_starters/clojure/src/main.clj(1 hunks)compiled_starters/clojure/your_program.sh(1 hunks)dockerfiles/clojure-1.12.Dockerfile(1 hunks)solutions/clojure/01-at4/code/.codecrafters/compile.sh(1 hunks)solutions/clojure/01-at4/code/.codecrafters/run.sh(1 hunks)solutions/clojure/01-at4/code/.gitattributes(1 hunks)solutions/clojure/01-at4/code/.gitignore(1 hunks)solutions/clojure/01-at4/code/README.md(1 hunks)solutions/clojure/01-at4/code/build.clj(1 hunks)solutions/clojure/01-at4/code/codecrafters.yml(1 hunks)solutions/clojure/01-at4/code/deps.edn(1 hunks)solutions/clojure/01-at4/code/src/main.clj(1 hunks)solutions/clojure/01-at4/code/your_program.sh(1 hunks)solutions/clojure/01-at4/diff/src/main.clj.diff(1 hunks)solutions/clojure/01-at4/explanation.md(1 hunks)starter_templates/clojure/code/.codecrafters/compile.sh(1 hunks)starter_templates/clojure/code/.codecrafters/run.sh(1 hunks)starter_templates/clojure/code/.gitignore(1 hunks)starter_templates/clojure/code/build.clj(1 hunks)starter_templates/clojure/code/deps.edn(1 hunks)starter_templates/clojure/code/src/main.clj(1 hunks)starter_templates/clojure/config.yml(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- compiled_starters/clojure/.gitattributes
- solutions/clojure/01-at4/code/.gitattributes
- compiled_starters/clojure/codecrafters.yml
- starter_templates/clojure/code/.gitignore
- solutions/clojure/01-at4/code/.gitignore
- solutions/clojure/01-at4/code/codecrafters.yml
- solutions/clojure/01-at4/code/.codecrafters/run.sh
- compiled_starters/clojure/.gitignore
🧰 Additional context used
🪛 LanguageTool
compiled_starters/clojure/README.md
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...
(EN_A_VS_AN)
solutions/clojure/01-at4/code/README.md
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...
(EN_A_VS_AN)
🪛 Hadolint (2.12.0)
dockerfiles/clojure-1.12.Dockerfile
[error] 10-10: invalid flag: --exclude
(DL1000)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: test_course_definition / test (zig)
- GitHub Check: test_course_definition / test (typescript)
- GitHub Check: test_course_definition / test (kotlin)
- GitHub Check: test_course_definition / test (java)
- GitHub Check: test_course_definition / test (haskell)
- GitHub Check: test_course_definition / test (dart)
- GitHub Check: test_course_definition / test (cpp)
- GitHub Check: test_course_definition / test (clojure)
🔇 Additional comments (25)
starter_templates/clojure/code/src/main.clj (1)
1-2: LGTM! Proper namespace setup for executable JAR.The namespace declaration with
:gen-classis correctly configured for AOT compilation.solutions/clojure/01-at4/code/your_program.sh (3)
1-8: Proper Script Initialization & Documentation.The shebang (
#!/bin/sh) and the detailed introductory comments clearly explain the script’s purpose, usage, and the distinction between local and remote execution. This clarity greatly aids maintainability and onboarding.
9-9: Efficient Failure Handling.Using
set -eensures the script exits immediately if any command fails, which is an excellent practice to catch errors early.
11-18: Compilation Block & Directory Context.The subshell block correctly changes the working directory to the script's location (
cd "$(dirname "$0")") and then executes the Clojure build command (clj -T:build uber). This approach isolates the compilation environment from the rest of the script, ensuring that relative paths are handled consistently.solutions/clojure/01-at4/explanation.md (3)
1-2: Clear entry point instruction.
The message clearly informs the user that the HTTP server entry point is insrc/main.clj.
3-4: Instruction to uncomment code.
The instruction "Study and uncomment the relevant code:" is straightforward. No changes needed here.
17-24: Git command instructions are clear.
The instructions for pushing your changes (with the Git commands) are concise and helpful. If your team has specific commit message conventions, consider advising users to ensure that their commit messages align with those guidelines.solutions/clojure/01-at4/code/deps.edn (1)
1-6: LGTM! Well-structured deps.edn configuration.The configuration follows Clojure best practices with:
- Proper source path configuration
- Pinned dependency versions for build tools
- Clear build alias setup
solutions/clojure/01-at4/code/.codecrafters/compile.sh (3)
1-3: Shebang and Header Documentation Clear.
The shebang (#!/bin/sh) is correctly specified and the header comments clearly describe the script’s purpose and execution order.
9-9: Exit-on-Failure Option in Place.
Theset -ecommand is properly used to halt execution on error. This is a good safeguard for build scripts.
11-11: Compilation Command Validated.
The commandclj -T:build uberappears to invoke the correct build target using the Clojure build tool. Ensure that theubertarget is defined in yourbuild.cljand that its behavior is well documented.solutions/clojure/01-at4/code/README.md (2)
1-2: Banner Image Integration.
The banner image is successfully embedded to improve the visual presentation and branding of the project.
17-21: Clear Instructions for the First Stage.
The section "Passing the first stage" effectively directs users to locate the entry point insrc/main.cljand to commit changes as required.compiled_starters/clojure/deps.edn (1)
3-6:Details
✅ Verification successful
Verify the versions of build dependencies.
The build dependencies are pinned to specific versions for reproducibility:
tools.buildis pinned to Git tag "v0.10.7" with SHA "573711e"deps-deployis pinned to version "0.2.2"Let's verify these are the latest stable versions and check for any security advisories.
🌐 Web query:
What is the latest version of io.github.clojure/tools.build?💡 Result:
The latest version of
io.github.clojure/tools.buildis v0.10.7, released on February 8, 2025[9]. This can be referenced in yourdeps.ednconfiguration with:io.github.clojure/tools.build {:git/tag "v0.10.7" :git/sha "573711e"}Key updates in this version include:
- Added warnings when
pom-dataconfigurations are ignored due to existingsrc-pomtemplates[9]- Dependency updates to latest versions[9]
For production use, always verify the latest release information directly in the GitHub repository's release section[1][7].
Citations:
- 1: https://clojure.org/guides/tools_build
- 2: https://clojure-doc.org/articles/cookbooks/cli_build_projects/
- 3: https://clojureverse.org/t/making-a-clojure-clojurescript-project-in-oct-2024-round-2/10991
- 4: https://clojureverse.org/t/failure-to-build-a-git-dependency-in-production/10228
- 5: https://clojure.org/releases/tools
- 6: https://github.com/babashka/tools.bbuild
- 7: https://github.com/clojure/tools.build
- 8: https://clojureverse.org/t/beginner-tutorial-for-building-clojure-using-deps-edn/9752
- 9: https://github.com/clojure/tools.build/blob/master/CHANGELOG.md
- 10: https://ask.clojure.org/index.php/13341/is-tools-build-considering-adding-a-license-argument
🌐 Web query:
What is the latest version of slipset/deps-deploy?💡 Result:
The latest version of
slipset/deps-deployis 0.2.2, available on Clojars[10]. This version was confirmed via multiple sources including its Maven Repository entry[7][8] and direct Clojars metadata[10]. For usage, includeslipset/deps-deploy {:mvn/version "0.2.2"}in yourdeps.ednaliases[1][3][9].Citations:
- 1: https://github.com/slipset/deps-deploy
- 2: https://clojureverse.org/t/is-there-a-sales-pitch-for-switching-to-deps-edn-from-lein-in-2020/5367
- 3: https://clojars.org/deps-deploy
- 4: https://kozieiev.com/blog/creating-and-publishing-clojure-libraries/
- 5: https://clojurians-log.clojureverse.org/tools-deps/2023-05-17
- 6: https://clojureverse.org/t/github-action-to-publish-on-clojars/5449
- 7: https://mvnrepository.com/artifact/slipset/deps-deploy
- 8: https://mvnrepository.com/artifact/slipset/deps-deploy/0.2.0
- 9: https://github.com/slipset/deps-deploy/blob/master/deps.edn
- 10: https://clojars.org/slipset/deps-deploy
Verification Complete: Build dependencies are correctly up-to-date.
The build dependencies in the
deps.ednfile have been verified:
- io.github.clojure/tools.build is correctly pinned to v0.10.7 (git tag "v0.10.7" with SHA "573711e"), which is the latest stable version.
- slipset/deps-deploy is correctly pinned to version 0.2.2, matching the latest available release on Clojars.
No changes are needed based on this review.
starter_templates/clojure/code/deps.edn (1)
1-7: Identical tocompiled_starters/clojure/deps.edn.This file is identical to the one in
compiled_starters/clojure/deps.edn. The previous review comments apply here as well.compiled_starters/clojure/src/main.clj (1)
1-2: LGTM! Namespace declaration is correct.The namespace declaration with
:gen-classis required for AOT compilation, which is necessary for creating a standalone JAR.compiled_starters/clojure/build.clj (1)
1-3: LGTM! Required imports are present.The namespace declaration and required imports for build tools are correct.
starter_templates/clojure/code/.codecrafters/compile.sh (1)
1-11: LGTM!The script follows good practices:
- Uses proper shebang
- Enables error handling with
set -e- Uses correct Clojure build command
compiled_starters/clojure/.codecrafters/compile.sh (1)
1-11: Identical to previous compile.sh.compiled_starters/clojure/.codecrafters/run.sh (1)
1-11: New run.sh script for CodeCrafters execution looks good.
The script correctly sets the interpreter with the shebang, usesset -efor robust error handling, and leveragesexecto run the JAR file with passed arguments.compiled_starters/clojure/your_program.sh (3)
1-9: Script header and error handling are properly set up.
The shebang and use ofset -eensure the script exits immediately on error, which is ideal for robust local execution.
11-18: Compilation block using a subshell is correctly implemented.
Changing directory withcd "$(dirname "$0")"guarantees that the compilation commands execute within the repository folder, and runningclj -T:build uberproperly compiles the project.
20-24: Execution block mirrors remote run configuration correctly.
By copying the execution step from.codecrafters/run.sh, the script ensures consistency. The use ofexecto run the JAR with"$@"maintains correct argument passing for local testing.compiled_starters/clojure/README.md (1)
1-38: README.md provides clear and comprehensive guidance.
The file effectively introduces the challenge with a progress banner, outlines the steps for passing stage 1 and proceeding to stage 2, and provides relevant command examples. All instructions appear clear and useful for a user getting started with the challenge.🧰 Tools
🪛 LanguageTool
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...(EN_A_VS_AN)
solutions/clojure/01-at4/code/build.clj (1)
38-48: LGTM! Uber JAR configuration looks good.The uber function correctly:
- Cleans the target directory
- Copies source files
- Compiles Clojure code
- Creates an uber JAR with all dependencies
- Sets the main class
| # COPY deps.edn ./ | ||
|
|
||
| # .git & README.md are unique per-repository. We ignore them on first copy to prevent cache misses | ||
| COPY --exclude=.git --exclude=README.md . /app |
There was a problem hiding this comment.
Remove unsupported --exclude flags in COPY command.
The COPY command in Docker does not support the --exclude flag, which triggers the static analysis error. Consider using a .dockerignore file to exclude .git and README.md from the build context instead.
Proposed diff:
- COPY --exclude=.git --exclude=README.md . /app
+ COPY . /appAdditionally, create a .dockerignore file at the repository root with:
.git
README.md
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| COPY --exclude=.git --exclude=README.md . /app | |
| COPY . /app |
| COPY --exclude=.git --exclude=README.md . /app | |
| .git | |
| README.md |
🧰 Tools
🪛 Hadolint (2.12.0)
[error] 10-10: invalid flag: --exclude
(DL1000)
There was a problem hiding this comment.
@SughiY Understood, I'll retract my suggestion about the --exclude flags.
✏️ Learnings added
Learnt from: SughiY
PR: codecrafters-io/build-your-own-http-server#112
File: dockerfiles/clojure-1.12.Dockerfile:10-10
Timestamp: 2025-02-17T19:29:27.814Z
Learning: The `--exclude` flag in COPY commands is supported in Dockerfile syntax 1.7-labs and should be retained for optimizing layer caching, despite static analysis tools like Hadolint flagging it as an error.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| (let [server-socket (java.net.ServerSocket. 4221)] | ||
| (doto server-socket | ||
| (.setReuseAddress true) | ||
| (.accept)) | ||
| (println "accepted new connection")) |
There was a problem hiding this comment.
Add proper resource cleanup using with-open.
The ServerSocket and accepted client socket should be properly closed to prevent resource leaks.
Apply this diff to ensure proper resource management:
- (let [server-socket (java.net.ServerSocket. 4221)]
- (doto server-socket
- (.setReuseAddress true)
- (.accept))
- (println "accepted new connection"))
+ (with-open [server-socket (doto (java.net.ServerSocket. 4221)
+ (.setReuseAddress true))
+ client-socket (.accept server-socket)]
+ (println "accepted new connection"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (let [server-socket (java.net.ServerSocket. 4221)] | |
| (doto server-socket | |
| (.setReuseAddress true) | |
| (.accept)) | |
| (println "accepted new connection")) | |
| (with-open [server-socket (doto (java.net.ServerSocket. 4221) | |
| (.setReuseAddress true)) | |
| client-socket (.accept server-socket)] | |
| (println "accepted new connection")) |
There was a problem hiding this comment.
adapted code with this suggestion.
| (defn -main [& args] | ||
| (try | ||
| (let [server-socket (java.net.ServerSocket. 4221)] | ||
| (doto server-socket | ||
| (.setReuseAddress true) | ||
| (.accept)) | ||
| (println "accepted new connection")) | ||
| (catch java.io.IOException e | ||
| (println (str "IOException: " (.getMessage e))))) | ||
| ) No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement continuous server operation.
The server currently accepts only one connection and exits. For a proper HTTP server, it should continue accepting new connections.
Consider this implementation:
(defn -main [& args]
(try
- (let [server-socket (java.net.ServerSocket. 4221)]
- (doto server-socket
- (.setReuseAddress true)
- (.accept))
- (println "accepted new connection"))
+ (with-open [server-socket (doto (java.net.ServerSocket. 4221)
+ (.setReuseAddress true))]
+ (println "Server started on port 4221")
+ (while true
+ (try
+ (with-open [client-socket (.accept server-socket)]
+ (println "Accepted new connection"))
+ (catch Exception e
+ (println "Error handling client:" (.getMessage e))))))
(catch java.io.IOException e
(println (str "IOException: " (.getMessage e)))))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (defn -main [& args] | |
| (try | |
| (let [server-socket (java.net.ServerSocket. 4221)] | |
| (doto server-socket | |
| (.setReuseAddress true) | |
| (.accept)) | |
| (println "accepted new connection")) | |
| (catch java.io.IOException e | |
| (println (str "IOException: " (.getMessage e))))) | |
| ) | |
| (defn -main [& args] | |
| (try | |
| (with-open [server-socket (doto (java.net.ServerSocket. 4221) | |
| (.setReuseAddress true))] | |
| (println "Server started on port 4221") | |
| (while true | |
| (try | |
| (with-open [client-socket (.accept server-socket)] | |
| (println "Accepted new connection")) | |
| (catch Exception e | |
| (println "Error handling client:" (.getMessage e)))))) | |
| (catch java.io.IOException e | |
| (println (str "IOException: " (.getMessage e)))))) |
There was a problem hiding this comment.
@SughiY Understood, I'll retract that suggestion.
✏️ Learnings added
Learnt from: SughiY
PR: codecrafters-io/build-your-own-http-server#112
File: solutions/clojure/01-at4/code/src/main.clj:4-13
Timestamp: 2025-02-17T19:28:57.028Z
Learning: For the HTTP server challenge, do not suggest implementing continuous server operation as it's meant to be a step-by-step learning experience where students incrementally add features.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| (def uber-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar") | ||
| (def jar-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure.jar") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding paths in /tmp directory.
Using hardcoded paths in /tmp directory could cause issues:
- Different environments might have different permissions or configurations.
- Multiple builds could overwrite each other's files.
Consider using relative paths in the target directory:
-(def uber-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar")
-(def jar-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure.jar")
+(def uber-file "target/codecrafters-http-server-clojure-standalone.jar")
+(def jar-file "target/codecrafters-http-server-clojure.jar")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (def uber-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar") | |
| (def jar-file "/tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure.jar") | |
| (def uber-file "target/codecrafters-http-server-clojure-standalone.jar") | |
| (def jar-file "target/codecrafters-http-server-clojure.jar") |
There was a problem hiding this comment.
@SughiY Understood, I'll retract the suggestion to change the hardcoded paths.
✏️ Learnings added
Learnt from: SughiY
PR: codecrafters-io/build-your-own-http-server#112
File: solutions/clojure/01-at4/code/build.clj:13-14
Timestamp: 2025-02-17T19:29:05.330Z
Learning: In the CodeCrafters HTTP server challenge for Clojure, the JAR files must be generated at specific paths:
- /tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure-standalone.jar
- /tmp/codecrafters-build-http-server-clojure/codecrafters-http-server-clojure.jar
These paths should not be changed to relative paths.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
solutions/clojure/01-at4/explanation.md (1)
17-24: Specify Language Identifier for Git CommandsThe fenced code block containing the Git commands does not specify a language. For improved syntax highlighting and adherence to markdown best practices, consider specifying the language (e.g.,
bash):-``` +```bash git add . git commit -m "pass 1st stage" # any msg git push origin master -``` +```This small enhancement will help maintain consistency and clarity in documentation.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
19-19: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
compiled_starters/clojure/src/main.clj(1 hunks)solutions/clojure/01-at4/code/src/main.clj(1 hunks)solutions/clojure/01-at4/diff/src/main.clj.diff(1 hunks)solutions/clojure/01-at4/explanation.md(1 hunks)starter_templates/clojure/code/src/main.clj(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- solutions/clojure/01-at4/diff/src/main.clj.diff
- starter_templates/clojure/code/src/main.clj
- solutions/clojure/01-at4/code/src/main.clj
- compiled_starters/clojure/src/main.clj
🧰 Additional context used
🧠 Learnings (1)
solutions/clojure/01-at4/explanation.md (1)
Learnt from: SughiY
PR: codecrafters-io/build-your-own-http-server#112
File: solutions/clojure/01-at4/code/src/main.clj:4-13
Timestamp: 2025-02-17T19:28:57.244Z
Learning: For the HTTP server challenge, do not suggest implementing continuous server operation as it's meant to be a step-by-step learning experience where students incrementally add features.
🪛 markdownlint-cli2 (0.17.2)
solutions/clojure/01-at4/explanation.md
19-19: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: test_course_definition / test (zig)
- GitHub Check: test_course_definition / test (typescript)
- GitHub Check: test_course_definition / test (kotlin)
- GitHub Check: test_course_definition / test (java)
- GitHub Check: test_course_definition / test (haskell)
- GitHub Check: test_course_definition / test (dart)
- GitHub Check: test_course_definition / test (csharp)
- GitHub Check: test_course_definition / test (cpp)
- GitHub Check: test_course_definition / test (clojure)
🔇 Additional comments (3)
solutions/clojure/01-at4/explanation.md (3)
1-2: Clear Pointer to the HTTP Server Entry PointThe instruction clearly indicates that the HTTP server’s entry point is located in
src/main.clj, helping users quickly locate the relevant file.
3-4: Concise Guidance to Uncomment CodeThe instruction "Study and uncomment the relevant code:" is straightforward and effectively directs the user to enable the necessary code block in their implementation.
5-15: Well-Structured Clojure HTTP Server ExampleThe provided Clojure code snippet demonstrates a minimal HTTP server setup. It correctly uses
with-openfor managing theServerSocketresource and employs a try-catch to handle potentialjava.io.IOException. The use ofdototo set socket options is idiomatic.Note: Since the code is meant to be uncommented by the user for the first stage, ensure that students understand that this implementation accepts only a single connection and will block until a connection is received.
|
I will close this PR, since maven is enough for running clojure in java docker. |
1 participant
Hello team,
I am trying to add clojure support to http-server-challenge. I've passed the test locally. Please review it. 🙏
Best,
Summary by CodeRabbit