-
Notifications
You must be signed in to change notification settings - Fork 119
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
OctaveCodeQuestion redux #786
base: main
Are you sure you want to change the base?
Conversation
The major piece missing as of now is a good set of unit tests. Does the CI know to pull a new Docker image |
@@ -376,7 +376,8 @@ | |||
|
|||
# A string containing the image ID of the docker image to be used to run | |||
# student Python code. Docker should download the image on first run. | |||
RELATE_DOCKER_RUNPY_IMAGE = "inducer/relate-runcode-python" |
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.
I'll have to revert this and the docker-build.sh
image renaming.
@@ -73,6 +73,7 @@ def _skip_real_docker_test(): | |||
REAL_RELATE_DOCKER_URL = "unix:///var/run/docker.sock" | |||
REAL_RELATE_DOCKER_TLS_CONFIG = None | |||
REAL_RELATE_DOCKER_RUNPY_IMAGE = "inducer/relate-runcode-python" | |||
REAL_RELATE_DOCKER_RUNOC_IMAGE = "davis68/relate-octave" |
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.
Should break out a local image of davis68 as well.
You can just add a |
To cut down on noise in my email, I'm unsubscribing from this PR. When it next needs my attention, please @-mention me or hit the "request review" button. Otherwise, I may not see your messages in a timely manner. |
@inducer OK, this Thanks to @karthiksharma98 for testing with me. |
BEFORE MERGING I need to revert a couple of local settings that are swept up in here but those are minor. |
@davis68 There seem to be quite a few CI failures still, and these need to be fixed before merging. Would you be able to address those? |
@inducer, @karthiksharma98 and I have handled all of the linting and basic unit tests issues. Now there's a slew of connection-related issues on the page tests. Can you advise on those? These may be Docker/CI—I set it to pull |
An easy way to see what's going on in an actions run is to (temporarily) include this action in the failing job steps list in |
I can see that no (?) Docker containers are starting and staying running for either Python or Octave. Since we didn't touch Python I have no idea why that would be the case. |
@inducer Can you advise on this? We're able to get the markdowns in markdowns.py to work in the page sandbox on a local RELATE installation, but for CI it doesn't seem to work. |
@inducer do old PRs not trigger the CI tests? |
They should... I'm not sure what's wrong there. |
This PR is a clean new build of OCQ on up-to-date RELATE. This will replace #633