[fix][package-management] Fix the new path /data introduced regression
#15367
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
--- Fixes apache#15362 *Motivation* The PR apache#13218 supports saving package data into filesystem. But it introduces an regression for the old versions. For example, we have a package function://public/default/[email protected], it will save the meta to the path function/public/default/package/v0.1/meta, and save the data to the path function/public/default/package/v0.1. By default, we are using distributed log as the package storage, and it supports saving data at a directory. But some storage like filesystem don't have the similar ability, it needs another path for saving data. This api provides the ability to support saving the data in another place. If you specify the data path as `/data`, the package will saved into function/public/default/package/v0.1/data. *Modifications* - make the data path configurable in the storage implementation.
zymap
added
type/bug
|
We have tests for the package service both bk storage and filesystem storage. It's a compatibility issue so I think it's hard to add a test for that. |
|
@nlu90 @eolivelli PTAL |
nlu90
approved these changes
Apr 28, 2022
|
ping @merlimat @eolivelli |
hangc0276
approved these changes
Apr 29, 2022
nicoloboschi
requested changes
Apr 29, 2022
There was a problem hiding this comment.
Good work!
We have tests for the package service both bk storage and filesystem storage. It's a compatibility issue so I think it's hard to add a test for that.
I believe that a unit test that ensures the default value (for different storage implementations) it would be very useful as we would have been able to spot the regression during the development of #13218
freeznet
approved these changes
Apr 30, 2022
HQebupt
approved these changes
May 2, 2022
murong00
approved these changes
May 5, 2022
|
@nicoloboschi PTAL. Thanks |
nicoloboschi
approved these changes
May 5, 2022
Closed
codelipenghui
pushed a commit
that referenced
this pull request
May 20, 2022
…ion (#15367) --- Fixes #15362 *Motivation* The PR #13218 supports saving package data into filesystem. But it introduces a regression for the old versions. For example, we have a package function://public/default/[email protected], it will save the meta to the path function/public/default/package/v0.1/meta, and save the data to the path function/public/default/package/v0.1. By default, we are using distributed log as the package storage, and it supports saving data in a directory. But some storage like filesystem doesn't have the similar ability, it needs another path for saving data. This API provides the ability to support saving the data in another place. If you specify the data path as `/data`, the package will be saved into function/public/default/package/v0.1/data. *Modifications* - make the data path configurable in the storage implementation. (cherry picked from commit aa9aa18)
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 23, 2022
…ion (apache#15367) --- Fixes apache#15362 *Motivation* The PR apache#13218 supports saving package data into filesystem. But it introduces a regression for the old versions. For example, we have a package function://public/default/[email protected], it will save the meta to the path function/public/default/package/v0.1/meta, and save the data to the path function/public/default/package/v0.1. By default, we are using distributed log as the package storage, and it supports saving data in a directory. But some storage like filesystem doesn't have the similar ability, it needs another path for saving data. This API provides the ability to support saving the data in another place. If you specify the data path as `/data`, the package will be saved into function/public/default/package/v0.1/data. *Modifications* - make the data path configurable in the storage implementation. (cherry picked from commit aa9aa18) (cherry picked from commit 46d6a7f)
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.
Fixes #15362
Motivation
The PR #13218 supports saving
package data into filesystem. But it introduces a regression for the
old versions.
For example, we have a package function://public/default/[email protected],
it will save the meta to the path function/public/default/package/v0.1/meta,
and save the data to the path function/public/default/package/v0.1.
By default, we are using distributed log as the package storage, and
it supports saving data in a directory.
But some storage like filesystem doesn't have the similar ability, it
needs another path for saving data.
This API provides the ability to support saving the data in another place.
If you specify the data path as
/data, the package will be saved intofunction/public/default/package/v0.1/data.
Modifications