Add delete_after_n method to When #108
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In situations where a component is expected to automatically retry an endpoint manually calling `delete` on a `Mock` may not be an option. To support this scenario, add a new `delete_after_n` method to `When` that will remove the `Mock` from the server after it has matched the specified number of calls. Related to alexliesenfeld#76 Related to alexliesenfeld#96
|
Thank you for your PR @wfchandler! Looks like a useful addition. I will have a closer look at it or get back to you soon! |
wfchandler
added a commit
to oxidecomputer/oxide.rs
that referenced
this pull request
Sep 16, 2024
From a user perspective uploading a disk is a single operation, but doing so via the SDK is a very involved process, involving stringing together multiple API calls with any interruption leaving the disk in a state that cannot be directly removed. Add a new `extras` module to the SDK that will contain convenience wrappers around actions that functions that are overly complicated. Create a `disk_import` function in `extras` that will perform a disk upload as a single step, and attempts to cleanup partially imported disks on failure. We make the `disk upload` CLI command a thin wrapper around the new SDK function, responsible only for cancellation and progress. We manually recreate the builder interface, as creating a procmacro to do this would require creating a new crate. If/when `extras` continues to grow we should reconsider this. `disk_import` also give callers access to a `DiskImportHandle` with a `watch` channel with the current number of bytes uploaded for use in progress tracking, and provides a `cancel` function that allows users to gracefully stop the disk upload. A disk that has been successfully created will not be removed, only those that are partially imported. The `cancel` function uses `tokio::select` to cancel the `Future` of the upload task. This should be safe in our in our case no locks are held across `await` points. No operations need to be safe to restart as we only call `tokio::select` a single time, they will be dropped immediately and losing a message is not a problem. We are forced to remove tests `test_disk_import_bulk_import_start_fail` and `test_disk_import_bulk_write_import_fail` as we now poll for disks in `Created` state, which may happen if a user immediately cancels a new request. This state is not eligible to be finalized, so we need to wait for the disk to progress to the next state, but `HttpMock` does not provide a way to programatically delete or update a mock after sending `n` responses. This means that the disk will always be returned as non-existing when querying, and cleanup will be skipped. I have opened alexliesenfeld/httpmock#108 to add a `delete_after_n` method, but this seems unlikely to be merged. We may want to consider moving to `wiremock` in the long term, which provides this functionality. Co-authored-by: Adam Leventhal <[email protected]>
wfchandler
added a commit
to oxidecomputer/oxide.rs
that referenced
this pull request
Sep 16, 2024
From a user perspective uploading a disk is a single operation, but doing so via the SDK is a very involved process, involving stringing together multiple API calls with any interruption leaving the disk in a state that cannot be directly removed. Add a new `extras` module to the SDK that will contain convenience wrappers around actions that functions that are overly complicated. Create a `disk_import` function in `extras` that will perform a disk upload as a single step, and attempts to cleanup partially imported disks on failure. We make the `disk upload` CLI command a thin wrapper around the new SDK function, responsible only for cancellation and progress. We manually recreate the builder interface, as creating a procmacro to do this would require creating a new crate. If/when `extras` continues to grow we should reconsider this. `disk_import` also give callers access to a `DiskImportHandle` with a `watch` channel with the current number of bytes uploaded for use in progress tracking, and provides a `cancel` function that allows users to gracefully stop the disk upload. A disk that has been successfully created will not be removed, only those that are partially imported. The `cancel` function uses `tokio::select` to cancel the `Future` of the upload task. This should be safe in our in our case no locks are held across `await` points. No operations need to be safe to restart as we only call `tokio::select` a single time, they will be dropped immediately and losing a message is not a problem. We are forced to remove tests `test_disk_import_bulk_import_start_fail` and `test_disk_import_bulk_write_import_fail` as we now poll for disks in `Created` state, which may happen if a user immediately cancels a new request. This state is not eligible to be finalized, so we need to wait for the disk to progress to the next state, but `HttpMock` does not provide a way to programatically delete or update a mock after sending `n` responses. This means that the disk will always be returned as non-existing when querying, and cleanup will be skipped. I have opened alexliesenfeld/httpmock#108 to add a `delete_after_n` method, but this seems unlikely to be merged. We may want to consider moving to `wiremock` in the long term, which provides this functionality. Co-authored-by: Adam Leventhal <[email protected]>
wfchandler
added a commit
to oxidecomputer/oxide.rs
that referenced
this pull request
Sep 16, 2024
From a user perspective uploading a disk is a single operation, but doing so via the SDK is a very involved process, involving stringing together multiple API calls with any interruption leaving the disk in a state that cannot be directly removed. Add a new `extras` module to the SDK that will contain convenience wrappers around actions that functions that are overly complicated. Create a `disk_import` function in `extras` that will perform a disk upload as a single step, and attempts to cleanup partially imported disks on failure. We make the `disk upload` CLI command a thin wrapper around the new SDK function, responsible only for cancellation and progress. We manually recreate the builder interface, as creating a procmacro to do this would require creating a new crate. If/when `extras` continues to grow we should reconsider this. `disk_import` also give callers access to a `DiskImportHandle` with a `watch` channel with the current number of bytes uploaded for use in progress tracking, and provides a `cancel` function that allows users to gracefully stop the disk upload. A disk that has been successfully created will not be removed, only those that are partially imported. The `cancel` function uses `tokio::select` to cancel the `Future` of the upload task. This should be safe in our in our case no locks are held across `await` points. No operations need to be safe to restart as we only call `tokio::select` a single time, they will be dropped immediately and losing a message is not a problem. We are forced to remove tests `test_disk_import_bulk_import_start_fail` and `test_disk_import_bulk_write_import_fail` as we now poll for disks in `Created` state, which may happen if a user immediately cancels a new request. This state is not eligible to be finalized, so we need to wait for the disk to progress to the next state, but `HttpMock` does not provide a way to programatically delete or update a mock after sending `n` responses. This means that the disk will always be returned as non-existing when querying, and cleanup will be skipped. I have opened alexliesenfeld/httpmock#108 to add a `delete_after_n` method, but this seems unlikely to be merged. We may want to consider moving to `wiremock` in the long term, which provides this functionality. Co-authored-by: Adam Leventhal <[email protected]>
wfchandler
added a commit
to oxidecomputer/oxide.rs
that referenced
this pull request
Sep 23, 2024
From a user perspective uploading a disk is a single operation, but doing so via the SDK is a very involved process, involving stringing together multiple API calls with any interruption leaving the disk in a state that cannot be directly removed. Add a new `extras` module to the SDK that will contain convenience wrappers around actions that functions that are overly complicated. Create a `disk_import` function in `extras` that will perform a disk upload as a single step, and attempts to cleanup partially imported disks on failure. We make the `disk upload` CLI command a thin wrapper around the new SDK function, responsible only for cancellation and progress. We manually recreate the builder interface, as creating a procmacro to do this would require creating a new crate. If/when `extras` continues to grow we should reconsider this. `disk_import` also give callers access to a `DiskImportHandle` with a `watch` channel with the current number of bytes uploaded for use in progress tracking, and provides a `cancel` function that allows users to gracefully stop the disk upload. A disk that has been successfully created will not be removed, only those that are partially imported. The `cancel` function uses `tokio::select` to cancel the `Future` of the upload task. This should be safe in our in our case no locks are held across `await` points. No operations need to be safe to restart as we only call `tokio::select` a single time, they will be dropped immediately and losing a message is not a problem. We are forced to remove tests `test_disk_import_bulk_import_start_fail` and `test_disk_import_bulk_write_import_fail` as we now poll for disks in `Created` state, which may happen if a user immediately cancels a new request. This state is not eligible to be finalized, so we need to wait for the disk to progress to the next state, but `HttpMock` does not provide a way to programatically delete or update a mock after sending `n` responses. This means that the disk will always be returned as non-existing when querying, and cleanup will be skipped. I have opened alexliesenfeld/httpmock#108 to add a `delete_after_n` method, but this seems unlikely to be merged. We may want to consider moving to `wiremock` in the long term, which provides this functionality. Co-authored-by: Adam Leventhal <[email protected]>
wfchandler
added a commit
to oxidecomputer/oxide.rs
that referenced
this pull request
Sep 23, 2024
From a user perspective uploading a disk is a single operation, but doing so via the SDK is a very involved process, involving stringing together multiple API calls with any interruption leaving the disk in a state that cannot be directly removed. Add a new `extras` module to the SDK that will contain convenience wrappers around actions that functions that are overly complicated. Create a `disk_import` function in `extras` that will perform a disk upload as a single step, and attempts to cleanup partially imported disks on failure. We make the `disk upload` CLI command a thin wrapper around the new SDK function, responsible only for cancellation and progress. We manually recreate the builder interface, as creating a procmacro to do this would require creating a new crate. If/when `extras` continues to grow we should reconsider this. `disk_import` also give callers access to a `DiskImportHandle` with a `watch` channel with the current number of bytes uploaded for use in progress tracking, and provides a `cancel` function that allows users to gracefully stop the disk upload. A disk that has been successfully created will not be removed, only those that are partially imported. The `cancel` function uses `tokio::select` to cancel the `Future` of the upload task. This should be safe in our in our case no locks are held across `await` points. No operations need to be safe to restart as we only call `tokio::select` a single time, they will be dropped immediately and losing a message is not a problem. We are forced to remove tests `test_disk_import_bulk_import_start_fail` and `test_disk_import_bulk_write_import_fail` as we now poll for disks in `Created` state, which may happen if a user immediately cancels a new request. This state is not eligible to be finalized, so we need to wait for the disk to progress to the next state, but `HttpMock` does not provide a way to programatically delete or update a mock after sending `n` responses. This means that the disk will always be returned as non-existing when querying, and cleanup will be skipped. I have opened alexliesenfeld/httpmock#108 to add a `delete_after_n` method, but this seems unlikely to be merged. We may want to consider moving to `wiremock` in the long term, which provides this functionality. Co-authored-by: Adam Leventhal <[email protected]>
wfchandler
added a commit
to oxidecomputer/oxide.rs
that referenced
this pull request
Sep 23, 2024
From a user perspective uploading a disk is a single operation, but doing so via the SDK is a very involved process, involving stringing together multiple API calls with any interruption leaving the disk in a state that cannot be directly removed. Add a new `extras` module to the SDK that will contain convenience wrappers around actions that functions that are overly complicated. Create a `disk_import` function in `extras` that will perform a disk upload as a single step, and attempts to cleanup partially imported disks on failure. We make the `disk upload` CLI command a thin wrapper around the new SDK function, responsible only for cancellation and progress. We manually recreate the builder interface, as creating a procmacro to do this would require creating a new crate. If/when `extras` continues to grow we should reconsider this. `disk_import` also give callers access to a `DiskImportHandle` with a `watch` channel with the current number of bytes uploaded for use in progress tracking, and provides a `cancel` function that allows users to gracefully stop the disk upload. A disk that has been successfully created will not be removed, only those that are partially imported. The `cancel` function uses `tokio::select` to cancel the `Future` of the upload task. This should be safe in our in our case no locks are held across `await` points. No operations need to be safe to restart as we only call `tokio::select` a single time, they will be dropped immediately and losing a message is not a problem. We are forced to remove tests `test_disk_import_bulk_import_start_fail` and `test_disk_import_bulk_write_import_fail` as we now poll for disks in `Created` state, which may happen if a user immediately cancels a new request. This state is not eligible to be finalized, so we need to wait for the disk to progress to the next state, but `HttpMock` does not provide a way to programatically delete or update a mock after sending `n` responses. This means that the disk will always be returned as non-existing when querying, and cleanup will be skipped. I have opened alexliesenfeld/httpmock#108 to add a `delete_after_n` method, but this seems unlikely to be merged. We may want to consider moving to `wiremock` in the long term, which provides this functionality. Co-authored-by: Adam Leventhal <[email protected]>
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In situations where a component is expected to automatically retry an endpoint, manually calling
deleteon aMockmay not be an option.To support this scenario, add a new
delete_after_nmethod toWhenthat will remove theMockfrom the server after it has matched the specified number of calls.Related to #76
Related to #96