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

Add test - delete an engine with a running build #127

Closed
johnml1135 opened this issue Sep 11, 2023 · 10 comments
Closed

Add test - delete an engine with a running build #127

johnml1135 opened this issue Sep 11, 2023 · 10 comments
Assignees
Labels
bug Something isn't working Test

Comments

@johnml1135
Copy link
Collaborator

It gave a few errors - it should close down gracefully.

Specifically, if a new engine is created and is in the process of being built (pending or active), the jobs should cancel before it is all deleted. There may also be a need for a state "deleting" ... maybe...

@johnml1135
Copy link
Collaborator Author

The abbreviated error message showing the need:

Call failed with gRPC error status. Status code: 'NotFound', Message: 'The build does not exist.'.\n","stream":"stdout","time":"2023-09-11T16:18:29.255840722Z","type":"info","source":"Grpc.Net.Client.Internal.GrpcCall"}
POST http://serval-api:81/serval.translation.v1.TranslationPlatformApi/BuildFaulted\n","stream":"stdout","time":"2023-09-11T16:18:29.215850556Z","type":"info","source":"System.Net.Http.HttpClient.TranslationPlatformApiClient.LogicalHandler"}
POST http://machine-engine/serval.translation.v1.TranslationEngineApi/Delete\n","stream":"stdout","time":"2023-09-11T16:18:27.478076473Z","type":"info","source":"System.Net.Http.HttpClient.Nmt.ClientHandler"}
POST http://serval-api:81/serval.translation.v1.TranslationPlatformApi/BuildStarted\n","stream":"stdout","time":"2023-09-11T16:17:51.818285896Z","type":"info","source":"System.Net.Http.HttpClient.TranslationPlatformApiClient.ClientHandler"}

@johnml1135 johnml1135 added the bug Something isn't working label Sep 11, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Serval Sep 11, 2023
@johnml1135 johnml1135 added this to the 1.2 Paratext Plugin MVP milestone Sep 11, 2023
@Enkidu93 Enkidu93 moved this from 🆕 New to 🔖 Ready in Serval Sep 13, 2023
@johnml1135
Copy link
Collaborator Author

may be related to #121 - about things happening that aren't quite complete.

@johnml1135
Copy link
Collaborator Author

Or, we disallow deletion on an engine if there is an active build and tell the user to cancel the build first.

@ddaspit
Copy link
Contributor

ddaspit commented Oct 5, 2023

This sounds like a bug. It should cancel the build automatically.

@johnml1135
Copy link
Collaborator Author

So, will we also need a "deleting" state? Or is that superfluous? Once it is deleted, there is no expectation of it existing. Yet, the cancelling and deleting may take 20 seconds to fully push through the whole system. How should this be handled?

@ddaspit
Copy link
Contributor

ddaspit commented Oct 5, 2023

We don't need a "deleting" state for builds. The "canceling" state should suffice. Right now, the delete request blocks until the current build has stopped and the engine was successfully deleted. I don't love this, but the alternative is to make the delete operation asynchronous, which means that we don't have a good way to report a failed delete operation back the calling application.

@johnml1135
Copy link
Collaborator Author

If that is the current behavior - then let's make sure that it is tested fully and I will be ok. We should be able to do this with unit tests at the Machine level. @ddaspit do you agree?

@ddaspit
Copy link
Contributor

ddaspit commented Oct 5, 2023

Yeah, sounds good.

@johnml1135 johnml1135 added the Test label Oct 5, 2023
@johnml1135 johnml1135 changed the title Delete an engine with a running build Add test - delete an engine with a running build Oct 5, 2023
@johnml1135
Copy link
Collaborator Author

To implement: Nmt and Smt test - delete the engine while the engine is building. Unit tests in Machine. May end up just being implemented in sillsdev/machine#101.

@Enkidu93
Copy link
Collaborator

Covered here and here.

@github-project-automation github-project-automation bot moved this from 🔖 Ready to ✅ Done in Serval Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Test
Projects
Status: ✅ Done
Development

No branches or pull requests

3 participants