Skip to content
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

Disable canvas and compilation #369

Merged
merged 3 commits into from
May 11, 2024
Merged

Conversation

frozenfrank
Copy link
Contributor

This PR continues efforts begun in #328 by adding guards to newly added method calls. It also leverages the newly added ApplicationProperties system to read in the values from a properties file.

@frozenfrank frozenfrank added the enhancement New feature or request label May 11, 2024
@frozenfrank frozenfrank requested a review from 19mdavenport May 11, 2024 05:45
@frozenfrank frozenfrank self-assigned this May 11, 2024
@frozenfrank
Copy link
Contributor Author

Note that the commit message "RUN_COMPILATION can be controlled via ApplicationProperties" in commit 50cb2c6 is mostly a theoretical question. Nobody has shown me nor demonstrated where the .properties file exists so I'm not sure how to actually use it.

@19mdavenport
Copy link
Collaborator

There is no .properties file associated with ApplicationProperties. It's tied to command line arguments

@frozenfrank
Copy link
Contributor Author

There is no .properties file associated with ApplicationProperties. It's tied to command line arguments

That explains it. I guess I just need some training on how to use the commandline arguments to adjust the values.

Is there something more you're looking for from this PR, or how can I help this one become approved as well?

Copy link
Collaborator

@19mdavenport 19mdavenport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add support for your new property to where it exists in main Server class, as well as providing it as an option in the docker compose file?

@frozenfrank
Copy link
Contributor Author

Could you add support for your new property to where it exists in main Server class, as well as providing it as an option in the docker compose file?

@19mdavenport I can look into the main Server class, but I have now idea how Docker works or while file I would be manipulating. I'll do my best.

@19mdavenport
Copy link
Collaborator

19mdavenport commented May 11, 2024

https://github.com/softwareconstruction240/autograder/blob/main/compose.yml you'd just need to add it with the rest of the command line arguments (lines 23-32)

@frozenfrank frozenfrank force-pushed the disable-canvas-and-compilation branch from 50cb2c6 to 9cedd8c Compare May 11, 2024 19:28
…ose file...

Add commandline flag --disable-run-compilaion

The Grader class checks this value freshly each time it is constructed.

Add stub to docker compose file for --disable-run-compilation

Rename --disable-run-compilation ==> --disable-compilation
@frozenfrank frozenfrank requested a review from 19mdavenport May 11, 2024 19:44
Copy link
Contributor Author

@frozenfrank frozenfrank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@19mdavenport I've added the support you asked for.

(I still don't understand everything in that file, but I was able to follow the pattern to add support for the property. Thanks for the tip!)

@frozenfrank frozenfrank merged commit 01b0e93 into main May 11, 2024
2 checks passed
@frozenfrank frozenfrank deleted the disable-canvas-and-compilation branch May 11, 2024 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants