Switch write path to transact in URLs #109
Merged
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
Codecov Report
@@ Coverage Diff @@
## master #109 +/- ##
==========================================
- Coverage 78.96% 75.38% -3.58%
==========================================
Files 29 29
Lines 832 902 +70
==========================================
+ Hits 657 680 +23
- Misses 175 222 +47
Continue to review full report at Codecov.
|
ttung
force-pushed
the
tonytung-url
branch
2 times, most recently
from
212472f to
fc624e8
Compare
shanaxel42
approved these changes
Jul 15, 2019
Previously, the read path for slicedimage operated with URLs. This allowed us to have manifests on local disk that referred to network resources for tiles. This PR will use URLs as the destination for manifest files and tile data. This PR also changes the way users can control the behavior of writing. Previously, we allowed for two callbacks: one allowed callers to designate where a sub-manifest is located on disk, and the other allowed callers to open a file for writing tile data. This turns out not to capture all the use cases for how we might want to control writing, so this PR creates WriterContract. It has three callbacks: one to designate where a sub-manifest is located in URL-space, another to designate where a tile is located in URL space, and one to write tile data to a URL. Furthermore, we provide a CompatibilityWriterContract that _mostly_ mimics the behavior of the old callbacks. The one notable exception is that the tricks utilized to perform in-place tile writing no longer work. CompatibilityWriterContract finds the destination in `tile_url_generator` by opening the tile data file, find out where it is on disk, and then closing the file. The new callback (write_tile_data) just opens the path and writes to it. There is not a way to maintain the file handle between `tile_url_generator` and `write_tile_data`. Because this is a slight change in behavior, we bump the version of the library to 4.0.0. Test plan: verified existing tests pass. verify existing starfish imagestack and experiment tests pass with this library (except for in-place tile construction). verify that the new in-place code for starfish passes with this library.
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.
Previously, the read path for slicedimage operated with URLs. This allowed us to have manifests on local disk that referred to network resources for tiles. This PR will use URLs as the destination for manifest files and tile data.
This PR also changes the way users can control the behavior of writing. Previously, we allowed for two callbacks: one allowed callers to designate where a sub-manifest is located on disk, and the other allowed callers to open a file for writing tile data. This turns out not to capture all the use cases for how we might want to control writing, so this PR creates WriterContract. It has three callbacks: one to designate where a sub-manifest is located in URL-space, another to designate where a tile is located in URL space, and one to write tile data to a URL.
Furthermore, we provide a CompatibilityWriterContract that mostly mimics the behavior of the old callbacks. The one notable exception is that the tricks utilized to perform in-place tile writing no longer work. CompatibilityWriterContract finds the destination in
tile_url_generatorby opening the tile data file, find out where it is on disk, and then closing the file. The new callback (write_tile_data) just opens the path and writes to it. There is not a way to maintain the file handle betweentile_url_generatorandwrite_tile_data.Because this is a slight change in behavior, we bump the version of the library to 4.0.0.
Test plan: verified existing tests pass. verify existing starfish imagestack and experiment tests pass with this library (except for in-place tile construction). verify that the new in-place code for starfish passes with this library.