-
Notifications
You must be signed in to change notification settings - Fork 227
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
Additional documentation for etc/hosts configuration issue #2273
Conversation
📝 WalkthroughWalkthroughThe pull request updates the Autolab installation documentation. In the docker-compose instructions, it now specifies that users accessing Autolab from a different host should add a line with their public IP, fully-qualified domain name, and the alias “autolab” to their Changes
Possibly related PRs
✨ Finishing Touches
🪧 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 (
|
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: 0
🧹 Nitpick comments (3)
docs/installation/docker-compose.md (1)
311-312
: Clarify instructions for non-localhost access.The newly added instruction instructs users accessing Autolab from a different host to update their
/etc/hosts
file accordingly. This addition improves clarity for non-local users. Consider providing an example or a brief note on how users might determine their public IP and FQDN if needed.docs/installation/tango-troubleshoot.md (2)
15-16
: Consider punctuation in the section header.The new section header
## Tango jobs completed but scores are not updated
is clear in intent. For improved readability, consider adding a comma before “but” (i.e., “## Tango jobs completed, but scores are not updated”). This is a stylistic suggestion and optional in markdown headers.🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....0.1:6379> ``` ## Tango jobs completed but scores are not updated If you are acces...(COMMA_COMPOUND_SENTENCE_2)
20-20
: Instruction for non-localhost users is well-defined.The new instruction for users accessing Autolab from a different host clearly outlines the required update to the
/etc/hosts
file. Ensure that users understand they must substitute<your public_ip>
and<your fqdn>
with their actual network values. This consistency with the updated Docker Compose documentation is beneficial.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/installation/docker-compose.md
(1 hunks)docs/installation/tango-troubleshoot.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/installation/tango-troubleshoot.md
[uncategorized] ~15-~15: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....0.1:6379> ``` ## Tango jobs completed but scores are not updated If you are acces...
(COMMA_COMPOUND_SENTENCE_2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
docs/installation/tango-troubleshoot.md (1)
16-18
: Explain the localhost limitation and fix.The added lines explain that when accessing Autolab via localhost, Tango sends autograder logs to its own localhost. This clarifies the root cause and provides the necessary fix by updating the
/etc/hosts
file. The explanation is concise and effective.
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.
LGTM!
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: 0
🧹 Nitpick comments (1)
app/controllers/assessment/autograde.rb (1)
235-237
: Good helper method for ID validationAdding a dedicated method for integer validation improves code organization and reusability.
However, consider renaming the method to follow Ruby naming conventions (avoid leading underscore for public methods) and add a documentation comment explaining its purpose:
-def _is_i?(string) +# Validates if a string represents a valid integer +def valid_integer?(string) !!(string =~ /\A[-+]?[0-9]+\z/) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/assets/javascripts/manage_submissions.js
(1 hunks)app/controllers/assessment/autograde.rb
(3 hunks)app/controllers/courses_controller.rb
(4 hunks)app/controllers/users_controller.rb
(1 hunks)
🔇 Additional comments (6)
app/assets/javascripts/manage_submissions.js (1)
161-169
: Good improvement to interactive problem scores!This change enhances the user experience by making problem scores clickable, transforming them from plain text values to interactive links that direct users to detailed feedback. The implementation also correctly handles undefined scores and formats scores to one decimal place.
app/controllers/users_controller.rb (1)
197-227
: Excellent enhancement for handling duplicate submission filenamesThis implementation properly handles duplicate filenames when users download multiple submissions. The two-phase approach first counts duplicates and then appends version numbers to ensure uniqueness, which prevents overwriting files with identical names in the zip archive.
app/controllers/courses_controller.rb (2)
556-559
: Improved roster warning displayGood enhancement to distinguish between errors and warnings during roster uploads, showing warnings to users without preventing the upload from completing successfully.
951-962
: Better duplicate email detection with line numbersThe implementation now tracks and reports duplicate emails with specific line numbers in the warning messages, making it easier for instructors to identify and fix issues in their roster files.
app/controllers/assessment/autograde.rb (2)
93-95
: Improved filtering of submission IDsThis change enhances robustness by using
filter_map
to ensure only valid submission IDs are processed for regrading, preventing potential errors from invalid input.
119-119
: More accurate success job countThe calculation now correctly uses the filtered submissions array size to determine success count, making the reporting more accurate.
Description
Adds clearer documentation for #2260.
The same documentation appears in troubleshooting of docker compose but I think searching in Tango's documentation when facing this issue is also an equally valid response.
How Has This Been Tested?
Ran
python3 -m mkdocs serve
.Types of changes
Checklist:
overcommit --install && overcommit --sign
to use pre-commit hook for linting