Project revamp and docker integration #251
Conversation
…g submission. updated tests accordingly
…sts. New utility functions for managing Docker Images. Checks for templates. Added filehandler List<Files> -> Zip
# Conflicts: # backend/app/src/main/java/com/ugent/pidgeon/util/CommonDatabaseActions.java
There was a problem hiding this comment.
Ik heb een aantal dingen aangepast en een aantal comments geplaatsts. Al bij al moest er veel veranderd worden dus is niet makkelijk om te reviewen maar de grote lijnen zien er zeker oké uit.
Momenteel heb ik vooral naar de code gekeken, ik zou vanavond nog eens wat situaties uitproberen ook om te zien of alles zeker werkt; Kan je me een idee geven van een script dat artifacts creëert zodat ik dat ook kan testen (er zal er vast één zijn bij de tests maar daar heb ik nog niet naar kunnen kijken
| // This docker test is configured in the simple mode (store test console logs) | ||
| return model.runSubmission(testScript); | ||
| }else{ | ||
| // This docker test is configured in the template mode (store json with feedback) | ||
| return model.runSubmissionWithTemplate(testScript, testTemplate); |
There was a problem hiding this comment.
Ksnap nie helemaal hoe de artifacts gekopieërd worden voordat het script gelopen wordt
There was a problem hiding this comment.
My bad dit is een logic error. Het kopieeren gebeurd 5 lines erboven maar mijn order of execution was verkeerd.
| } | ||
| submission.setTestFinished(true); |
There was a problem hiding this comment.
Dit is niet helemaal hoe ik het in gedachten had. Dit heeft nu vrij weinig nut want op het moment dat de frontend de response krijgt is de volledige dockertest dus al gerund en zal dit veld dus altijd true zijn. Tis eerder de bedoeling dat er een reponse wordt teruggegeven waar dit dus default op false staat en dat dan in de achtergrond de dockertesten gerunt worden & dit veld geupdate wordt. Op deze manier kan de user al feedback krijgen over de structure test terwijl de docker testen aan het runnen zijn.
| // delete test entry | ||
| if(httpMethod.equals(HttpMethod.DELETE)){ | ||
| // first check if docker image is not used anywhere else | ||
| if(!testRepository.imageIsUsed(dockerImage)){ | ||
| // image is no longer required for any tests | ||
| DockerSubmissionTestModel.removeDockerImage(dockerImage); | ||
| } | ||
|
|
||
| // Create/update test entity | ||
| TestEntity test = new TestEntity(dockerImage, dockertestFileEntityId, structuretestFileEntityId); | ||
| test = testRepository.save(test); | ||
| projectEntity.setTestId(test.getId()); | ||
| // delete test | ||
| testRepository.deleteById(testEntity.getId()); | ||
| projectEntity.setTestId(null); | ||
| projectRepository.save(projectEntity); | ||
| return ResponseEntity.ok(entityToJsonConverter.testEntityToTestJson(test, projectId)); | ||
| } catch (IOException e) { | ||
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body("Error while saving files: " + e.getMessage()); | ||
| return ResponseEntity.ok().build(); |
There was a problem hiding this comment.
Volgens mij mag dit gewoon weg, dit heeft heeft een aparte route (helemaal onderaan dit bestand) die je correct hebt aangepast opt eerste gezicht dus dit hoeft hier niet denkik
| if(!(dockerImage == null && dockerScript == null && dockerTemplate == null)){ | ||
|
|
||
| // update/install image if possible. | ||
| DockerSubmissionTestModel.installImage(dockerImage); |
There was a problem hiding this comment.
Is dit is wat mogelijks een tijdje kan duren? Het lijkt me niet handig dat er gewacht moet worden op een response indien dit installeren even duurt
There was a problem hiding this comment.
Inderdaad dit kan wel een tijdje duren. Ik zal dit in een apparte thread doen
| DockerClientConfig config = DefaultDockerClientConfig.createDefaultConfigBuilder() | ||
| .withDockerHost("tcp://10.5.0.4:2375").build(); | ||
|
|
||
| DockerClientConfig config = DefaultDockerClientConfig.createDefaultConfigBuilder().build(); |
There was a problem hiding this comment.
kweet niet of dit de bedoeling was, was dit niet net aangepast om dind te werken? Of werkt het dan niet lokaal?
There was a problem hiding this comment.
Dit was wel de bedoeling, deze code van arthur werkte niet meer en hij moest hier nog een mail voor sturen. Op deze manier werkt het lokaal nog.
|
Feedback commit gedaan. |
…st one Test is required. Added docker image check to testCheck
…m/SELab-2/UGent-6 into project_revamp_docker_integration
TestenOnderdelen van een test
Aanpassen testen v/e projectDe url voor volgende request is altijd Toevoegen test
Aanpassen v/e test
Verwijderen
Disclaimer: Indien het de eerste keer is dat een dockerimage gebruikt wordt bij een test moet deze nog geinstalleerd worden. Een submission erna zal dus niet correct draaien en de testen falen (zie later). Misschien handig om dit ook ergens te vermelden Opvragen van de testenEen {
"projectUrl": "/api/projects/1",
"dockerImage": "fedora:latest",
"dockerScript": "output=$(bash /shared/input/helloworld.sh); \nif [[ \"$output\" == 'Hello World' ]]; \n\tthen echo 'Test one is successful'; \n\techo 'PUSH ALLOWED' > /shared/output/testOutput; \nelse \n\techo 'PUSH DENIED'; > /shared/output/testOutput; \nfi",
"dockerTemplate": "@helloworld\n>required\n>description=\"Helloworldtest\"\nHello World\n",
"structureTest": "helloworld.sh"
}Deze velden zijn
VoorbeeldtestenDockerimage:
Dockerscript:Simple: output=$(bash /shared/input/helloworld.sh);
if [[ "$output" == 'Hello World' ]];
then echo 'Test one is successful';
echo 'PUSH ALLOWED' > /shared/output/testOutput;
else
echo 'PUSH DENIED'; > /shared/output/testOutput;
fiTemplate: output=$(bash /shared/input/helloworld.sh);
echo "$output" > /shared/output/helloworldDockertemplate:gebruikt volgende syntax, de 2 lijntjes met Bijvoorbeeld: structuretest
Submission feedbackAlgemene response{
"submissionId": 32,
"projectUrl": "/api/projects/1",
"groupUrl": "/api/groups/1",
"projectId": 1,
"groupId": 1,
"fileUrl": "/api/submissions/32/file",
"structureAccepted": true,
"dockerStatus": "finished",
"submissionTime": "2024-05-09T18:52:49Z",
"structureFeedback": "File structure is correct",
"dockerFeedback": {
"type": "TEMPLATE",
"feedback": {
"subtests": [
{
"testName": "helloworld",
"testDescription": "Helloworldtest",
"correct": "Hello World\n",
"output": "Hello World\n",
"required": true,
"succes": true
},
],
"allowed": true
},
"artifactUrl": "/api/submissions/32/artifacts"
}Structure feedback:
Docker feedback:
Artifacts:
|
|
@Aqua-sc Misschien is het ook belangrijk om bij beide test script en image toe te voegen dat de container altijd based op een linux distro moet zijn. En dat de test script in bash gebeurd. Het is wel mogelijk via bash python commandos etc te doen op een python container bvb |
|
Klopt het dat ik een POST request moet sturen naar
Betekend dit dat 1 tab == 4 spaties (zoals in het voorbeeld) of 8 spaties (in frontend is 1 tab == 8 spaties voor één of andere reden, )? Momenteel is 1 tab gelijk aan 1 spatie in mijn structure parser, dus dit moet ik nog aanpassen. |
Niet noodzakelijk. Bij POST/PUT wordt alles wat je meestuurt in de databank toegevoegd/vervangen. Indien je dan
Ik zal antwoorden hoe ik het momenteel begrijp maar mss kan @TR1VER dit nog bevestigen: zolang dat het aantal spaties consistent is maakt het niet uit hoeveel er zijn. (De eerste indent bepaalt dus hoeveel spaties er gebruikt worden). Ik weet niet of spaties en tabs door elkaar gebruikt kunnen worden. |
|
@usserwoutV2 voor de structure test is het eigenlijk enkel belangrijk dat de spaties/tabs consistent zijn. Zolang dit zo is mag de user beide spaties of tabs gebruiken. Op het einde van een folder moet er ook een / staan. |
Aqua-sc
changed the title
In this pull request, the following were changed/added