-
Notifications
You must be signed in to change notification settings - Fork 5
Athena: Fix/update broken dependencies
#399
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR updates infrastructure and dependencies across multiple modules, including broadened HTTP timeout exception handling, Docker build context configuration changes, Python dependency additions (python-dotenv), Dockerfile build step modifications, protobuf version upgrades, and a Next.js version bump. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@athena/docker-compose.yml`:
- Around line 11-19: The llm_core service contains inconsistent indentation for
the build: block; adjust the indentation so the build:, context:,
additional_contexts:, and its nested keys align with the same indentation level
used by other services (match 4-space indentation used elsewhere) in the
llm_core service definition to restore consistent YAML formatting for the build
block.
In `@athena/modules/text/module_text_cofee/Dockerfile`:
- Around line 5-7: Update the Dockerfile to document the protobuf version
synchronization: next to the protobuf-dev=31.1-r1 pin (and/or the golang:alpine
FROM line) add a brief comment referencing pyproject.toml's protobuf==6.31.1
requirement so maintainers understand the versions must stay in sync, and
optionally replace golang:alpine with a pinned base like golang:1.23-alpine3.21
to avoid drift.
🧹 Nitpick comments (3)
athena/modules/text/module_text_llm/Dockerfile (1)
22-32: Optional: avoid installing dev dependencies in the runtime image.If dev/test groups aren’t required at runtime, you can reduce image size by installing only main deps.
♻️ Suggested tweak (optional)
- && poetry install --no-interaction --no-ansi --no-root + && poetry install --no-interaction --no-ansi --no-root --only main @@ -RUN poetry install --no-interaction --no-ansi +RUN poetry install --no-interaction --no-ansi --only mainathena/assessment_module_manager/assessment_module_manager/endpoints/health_endpoint.py (1)
11-18: Consider catching all httpx timeouts, not just connect timeouts.
httpx.ReadTimeout/WriteTimeout/PoolTimeoutstill propagate and could 500 the health check. Catchinghttpx.TimeoutExceptionkeeps “unreachable” handling consistent.♻️ Suggested tweak
- except (httpx.ConnectError, httpx.ConnectTimeout): + except (httpx.ConnectError, httpx.TimeoutException):athena/modules/programming/module_programming_llm/Dockerfile (1)
3-3: Nit: Stale comment in header.The comment says "assessment module manager" but this is
module_programming_llm. Consider updating for accuracy.Suggested fix
-# This is the Dockerfile for the assessment module manager. +# This is the Dockerfile for the programming exercise feedback LLM module.
| llm_core: | ||
| build: ./llm_core | ||
| build: | ||
| context: ./llm_core | ||
| additional_contexts: | ||
| athena: service:athena | ||
| depends_on: | ||
| - athena | ||
| image: llm_core | ||
| command: echo "llm_core build succeeded, exiting (this is normal)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation in llm_core service.
The build: block has extra indentation (6 spaces) compared to other services (4 spaces). This inconsistency should be fixed for readability and consistency.
Suggested fix
llm_core:
- build:
- context: ./llm_core
- additional_contexts:
- athena: service:athena
- depends_on:
+ build:
+ context: ./llm_core
+ additional_contexts:
+ athena: service:athena
+ depends_on:
- athena
- image: llm_core
- command: echo "llm_core build succeeded, exiting (this is normal)"
+ image: llm_core
+ command: echo "llm_core build succeeded, exiting (this is normal)"📝 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.
| llm_core: | |
| build: ./llm_core | |
| build: | |
| context: ./llm_core | |
| additional_contexts: | |
| athena: service:athena | |
| depends_on: | |
| - athena | |
| image: llm_core | |
| command: echo "llm_core build succeeded, exiting (this is normal)" | |
| llm_core: | |
| build: | |
| context: ./llm_core | |
| additional_contexts: | |
| athena: service:athena | |
| depends_on: | |
| - athena | |
| image: llm_core | |
| command: echo "llm_core build succeeded, exiting (this is normal)" |
🤖 Prompt for AI Agents
In `@athena/docker-compose.yml` around lines 11 - 19, The llm_core service
contains inconsistent indentation for the build: block; adjust the indentation
so the build:, context:, additional_contexts:, and its nested keys align with
the same indentation level used by other services (match 4-space indentation
used elsewhere) in the llm_core service definition to restore consistent YAML
formatting for the build block.
| # Protobuf build | ||
| FROM golang:alpine AS cofee_protobuf | ||
| RUN apk update && apk add --no-cache make protobuf-dev=29.4-r0 | ||
| RUN apk update && apk add --no-cache make protobuf-dev=31.1-r1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the pinned protobuf-dev version appears in the Dockerfile
rg -n "protobuf-dev=31\.1-r1" athena/modules/text/module_text_cofee/DockerfileRepository: ls1intum/edutelligence
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Check the complete Dockerfile to understand the full context
cat -n athena/modules/text/module_text_cofee/DockerfileRepository: ls1intum/edutelligence
Length of output: 1344
🏁 Script executed:
#!/bin/bash
# Search for any other base image pins or Alpine version specifications in the repo
rg -i "golang:alpine|alpine:" --type dockerfile | head -20Repository: ls1intum/edutelligence
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Search for Dockerfile patterns in the repository
find . -name "Dockerfile*" -type f | head -20Repository: ls1intum/edutelligence
Length of output: 901
🏁 Script executed:
#!/bin/bash
# Check other Dockerfile patterns to see if this is a consistent issue
rg "FROM.*alpine" | head -20Repository: ls1intum/edutelligence
Length of output: 405
🏁 Script executed:
#!/bin/bash
# Check if there are any other pinned package versions in Dockerfiles for comparison
rg "apk add.*=" --type dockerfile | head -20Repository: ls1intum/edutelligence
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Search for any issues, discussions, or TODOs related to Alpine/package versions
rg -i "alpine|protobuf-dev|31\.1-r1" --type-add 'yaml:*.yml' --type yaml | head -20Repository: ls1intum/edutelligence
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Check git history to see if this was recently added or if there are related commits
git log --oneline --all -- "athena/modules/text/module_text_cofee/Dockerfile" | head -10Repository: ls1intum/edutelligence
Length of output: 190
🏁 Script executed:
#!/bin/bash
# Check the specific commit that fixed the outdated dependency
git show 6658f9b --statRepository: ls1intum/edutelligence
Length of output: 50379
🏁 Script executed:
#!/bin/bash
# Check the detailed changes in that commit
git show 6658f9b -- athena/modules/text/module_text_cofee/DockerfileRepository: ls1intum/edutelligence
Length of output: 1579
🏁 Script executed:
#!/bin/bash
# Check if there are any Alpine version constraints or base image pins elsewhere in the repo
rg "FROM golang:alpine" -A 5 -B 5Repository: ls1intum/edutelligence
Length of output: 877
🏁 Script executed:
#!/bin/bash
# Verify if the pinned version solves a critical issue by checking dependencies
cat -n athena/modules/text/module_text_cofee/pyproject.toml | head -30Repository: ls1intum/edutelligence
Length of output: 1257
Document the protobuf version synchronization requirement in the Dockerfile.
The version pin protobuf-dev=31.1-r1 on a floating golang:alpine base image creates a maintenance risk—when the base image updates, this specific package version may become unavailable. However, this pin is intentionally synchronized with the Python protobuf==6.31.1 dependency (documented in pyproject.toml) following a recent fix. To make this constraint clearer to future maintainers, add a comment in the Dockerfile linking to the pyproject.toml requirement, or consider pinning the golang image to a specific Alpine version (e.g., golang:1.23-alpine3.21) to reduce drift.
🤖 Prompt for AI Agents
In `@athena/modules/text/module_text_cofee/Dockerfile` around lines 5 - 7, Update
the Dockerfile to document the protobuf version synchronization: next to the
protobuf-dev=31.1-r1 pin (and/or the golang:alpine FROM line) add a brief
comment referencing pyproject.toml's protobuf==6.31.1 requirement so maintainers
understand the versions must stay in sync, and optionally replace golang:alpine
with a pinned base like golang:1.23-alpine3.21 to avoid drift.
Athena Test Results Summary
Failing Tests Summary
|
||||||||||||||
📊 Detailed Coverage TableCombining 3 coverage files... 📊 Combined Coverage Summary:
Total packages: 31 Note: Coverage thresholds: ✅ (≥70%), ❌ (<70%) |
Athena: Fix/update broken dependencies
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
Motivation and Context
Trying to run Athena without any changes leads to errors. this PR fixes those errors.
Description
dotenvas a dependency for modules that need itmodule_text_cofeeto fix runtime errorSteps for Testing
As the weren't made any changes to functionality, testing only needs to consist of validating that every service starts and runs as intended
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Screenshots
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.