-
Notifications
You must be signed in to change notification settings - Fork 1
135 stop making OpenAI and redis credentials required in the autograderrequest #146
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
135 stop making OpenAI and redis credentials required in the autograderrequest #146
Conversation
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.
Pull request overview
This PR refactors credential management by removing OpenAI and Redis credentials from the AutograderRequest model and moving them to environment variables. The approach allows teachers to configure OpenAI API keys via repository environments while Redis credentials are set as organization secrets in GitHub Actions.
Key Changes:
- Removed credential fields (
openai_key,redis_url,redis_token) fromAutograderRequestmodel - Modified GitHub Actions adapter to set credentials as environment variables instead of passing them through the request
- Updated
autograder_facade.pyto read credentials from environment variables for AI feedback and Redis operations
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
connectors/models/autograder_request.py |
Removed OpenAI and Redis credential fields from the model |
connectors/port.py |
Removed credential parameters from the abstract create_request method |
connectors/adapters/github_action_adapter/github_adapter.py |
Sets credentials as environment variables in create_request method |
connectors/adapters/github_action_adapter/github_entrypoint.py |
Added validation for OpenAI key when using AI feedback mode; updated parameter descriptions |
connectors/adapters/api/api_adapter.py |
Removed credential parameters from create_request method |
connectors/adapters/api/api_entrypoint.py |
Removed credential parameters from the API endpoint |
autograder/autograder_facade.py |
Reads credentials from environment variables for AI feedback validation and Redis operations |
action.yml |
Updated to reference Redis credentials as secrets; clarified OpenAI key requirement |
tests/unit/test_facade.py |
Updated test to remove openai_key parameter and adjusted error message assertion |
tests/playroom/essay_playroom.py |
Removed openai_key parameter from request creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
action.yml
Outdated
| REDIS_URL: ${{ secrets.REDIS_URL }} | ||
| REDIS_TOKEN: ${{ secrets.REDIS_TOKEN }} |
Copilot
AI
Dec 23, 2025
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.
The syntax for accessing secrets in GitHub Actions is incorrect. Secrets cannot be accessed directly in the env section of action.yml using ${{ secrets.REDIS_URL }} and ${{ secrets.REDIS_TOKEN }}.
In GitHub Actions, secrets need to be passed through inputs first, or accessed via the workflow file. The action.yml file can only access inputs, not secrets directly. The workflow that uses this action would need to pass the secrets as inputs.
You should either:
- Add redis_url and redis_token as inputs (similar to openai_key) and let the workflow pass the secrets, OR
- Document that these must be set as repository/organization secrets and accessed by the workflow before being passed to the action
| if not args.openai_key: | ||
| raise ValueError("OpenAI API key is required for AI feedback mode in GitHub Actions. Please configure OPENAI_API_KEY as a secret.") |
Copilot
AI
Dec 23, 2025
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.
The validation for AI feedback mode only checks for the OpenAI key but not for Redis credentials. According to the error message in autograder_facade.py (line 218), both Redis URL and Redis token are also required for AI feedback mode.
The validation should check all three credentials (openai_key, redis_url, and redis_token) to fail fast and provide a clear error message to users before attempting to run the grading process.
| if not args.openai_key: | |
| raise ValueError("OpenAI API key is required for AI feedback mode in GitHub Actions. Please configure OPENAI_API_KEY as a secret.") | |
| missing_credentials = [] | |
| if not args.openai_key: | |
| missing_credentials.append("OpenAI API key (OPENAI_API_KEY)") | |
| if not args.redis_url: | |
| missing_credentials.append("Redis URL (REDIS_URL)") | |
| if not args.redis_token: | |
| missing_credentials.append("Redis token (REDIS_TOKEN)") | |
| if missing_credentials: | |
| missing_str = ", ".join(missing_credentials) | |
| raise ValueError( | |
| f"AI feedback mode requires the following credentials: {missing_str}. " | |
| "Please configure them as GitHub Actions secrets." | |
| ) |
| self.assertEqual(response.status, "fail") | ||
| self.assertEqual(response.final_score, 0.0) | ||
| self.assertIn("OpenAI key, Redis URL, and Redis token are required", response.feedback) | ||
| self.assertIn("OpenAI key, Redis URL, and Redis token are required for AI feedback mode", response.feedback) |
Copilot
AI
Dec 23, 2025
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.
The error message text differs between the test expectation and the actual error message in the code. The test expects "OpenAI key, Redis URL, and Redis token are required for AI feedback mode" but the actual error message in autograder_facade.py line 218 is "OpenAI key, Redis URL, and Redis token are required for AI feedback mode. These must be set as environment variables."
The test assertion should match the complete error message from the implementation.
| self.assertIn("OpenAI key, Redis URL, and Redis token are required for AI feedback mode", response.feedback) | |
| self.assertIn("OpenAI key, Redis URL, and Redis token are required for AI feedback mode. These must be set as environment variables.", response.feedback) |
| openai_key: | ||
| description: 'Optional OpenAI API key for generating AI-assisted feedback.' | ||
| required: false | ||
| redis_url: | ||
| description: 'Optional URL for a Redis instance for caching.' | ||
| required: false | ||
| redis_token: | ||
| description: 'Optional authentication token for the Redis instance.' | ||
| description: 'OpenAI API key for AI feedback. REQUIRED for AI feedback mode in GitHub Actions.' |
Copilot
AI
Dec 23, 2025
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.
The description is unclear about what credentials are required and when. It should specify:
- That the OpenAI API key is REQUIRED (not optional) when using AI feedback mode
- That Redis credentials are also required for AI feedback mode
- How to configure these in GitHub Actions (e.g., as repository secrets)
This would help users understand the setup requirements better.
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.
There are some important things to clear out.
The core idea is:
- For the API entrypoint, no single credential is required (the reason for api auth) [you're doing it right]
- For the github entrypoint we'll do the same, except for the OpenAI key. So we don't need users to add their redis credentials anymore. (No need for authentication)
IMPORTANT: When I mentioned about the organization secrets, i was just talking about how users may store their secret variables, there's no need to change anything about that. We still receive those variables as action inputs (the way it currently is). But if you take a look at how it works in acr-wonderland repos, for example, you'll see that these secrets are sent to the action inputs via de organization secrets (the user who sets the action workflow for a repo is responsible for this, we as devs for the action itself are not) .
So the only changes you'll have to do in the action parameters (action.yml, entrypoint.sh, api_entrypoint.sh) is to remove the redis credentials (and furtherly add a new way of acessing them)
I'm thinking about an even better approach for that since we'll now have a service completely responsible for security. It might be nice to have a way to still use our own OpenAI key in the github environment but not use it directly. The idea would be to use the security api as a proxy letting it know that we're trying to make an API call from a github action workflow. (By doing that, we can include rate limit per user or even subscriptions)
In summary:
The main changes to be done here is to change the github actions logic.
- Remove redis credentials
- Keep OpenAI key
- Keep the way that action inputs are handled (
inputs.*)
| if openai_key: | ||
| os.environ["OPENAI_API_KEY"] = openai_key |
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.
This is already enough. We're going to already have our own redis credentials (from api-grader-security)
| if redis_url: | ||
| os.environ["REDIS_URL"] = redis_url | ||
| if redis_token: | ||
| os.environ["REDIS_TOKEN"] = redis_token |
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.
You can remove that.
action.yml
Outdated
| REDIS_URL: ${{ secrets.REDIS_URL }} | ||
| REDIS_TOKEN: ${{ secrets.REDIS_TOKEN }} |
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.
1 - This won't work because it cannot access the organization secret variables.
2 - This is not necessary since we won't ask for redis credentials anymore
Finished implementing the requested changes in my PR
…-required-in-the-autograderrequest
From what I understood from our call, I decided to use this approach: the teacher sets up a repository environment in GitHub Actions with their OpenAI API key, while the Redis credentials are configured as organization secrets.