util/config: fix #1956 config error printing #1957
Conversation
src/util/config.rs
Outdated
| "no" => Ok(DBCompressionType::DBNo), | ||
| "snappy" => Ok(DBCompressionType::DBSnappy), | ||
| "zlib" => Ok(DBCompressionType::DBZlib), | ||
| "bzip2" => Ok(DBCompressionType::DBBz2), | ||
| "lz4" => Ok(DBCompressionType::DBLz4), | ||
| "lz4hc" => Ok(DBCompressionType::DBLz4hc), | ||
| _ => Err(ConfigError::RocksDB), | ||
| "zstd" => Ok(DBCompressionType::DBZstd), | ||
| v => Err(ConfigError::Value(format!("cannot parse {:?} as a compression type", v))), |
There was a problem hiding this comment.
"invalid compression type {:?}"
|
|
||
| Ok(result) | ||
| tp.split(':') | ||
| .map(parse_rocksdb_compression) |
There was a problem hiding this comment.
It won't compile. And format is broken.
There was a problem hiding this comment.
I made a mistake, it should be fine here.
|
PTAL @BusyJay |
src/util/config.rs
Outdated
| "no" => Ok(DBCompressionType::DBNo), | ||
| "snappy" => Ok(DBCompressionType::DBSnappy), | ||
| "zlib" => Ok(DBCompressionType::DBZlib), | ||
| "bzip2" => Ok(DBCompressionType::DBBz2), | ||
| "lz4" => Ok(DBCompressionType::DBLz4), | ||
| "lz4hc" => Ok(DBCompressionType::DBLz4hc), | ||
| _ => Err(ConfigError::RocksDB), | ||
| "zstd" => Ok(DBCompressionType::DBZstd), | ||
| v => Err(ConfigError::Value(format!("invalid compression type {:?}", v))), |
There was a problem hiding this comment.
reject.
won't fix and will wait all other vs in codebase.
There was a problem hiding this comment.
v is a temporary variable during processing, using tp is more clear and straight forward for user. You can raise a pr to clean up all the other places. But since this pr is about to fix error printing, so it's fine to fix them all in this pr too.
There was a problem hiding this comment.
any update here?
Maybe we can keep it for now and use another PR to clean up all?
There was a problem hiding this comment.
Just checked out the diff, places need to be changed are L54 and L128, all are introduced in this pr.
|
Ping @andelf |
|
PTAL @BusyJay |
|
LGTM |
Fixes #1956
PTAL @zhangjinpeng1987 @siddontang