Optimize file path builder and have separate handler for streaming file #29
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
sarankk
force-pushed
the
move-stream-handler
branch
5 times, most recently
from
b9d98ad
to
2be5364
Compare
sarankk
force-pushed
the
move-stream-handler
branch
2 times, most recently
from
2c3bbcf
to
2f56005
Compare
yifan-c
reviewed
Jun 10, 2022
| /** | ||
| * Contains the keyspace and table name in Cassandra | ||
| */ | ||
| public class CassandraQualifiedName |
There was a problem hiding this comment.
weak nit: QualifiedTableName seems more fit.
There was a problem hiding this comment.
:( I liked CassandraQualifiedName, but either is fine I guess
common/src/main/java/org/apache/cassandra/sidecar/common/utils/ValidationUtils.java
Outdated
Show resolved
Hide resolved
yifan-c
approved these changes
Jun 10, 2022
frankgh
reviewed
Jun 11, 2022
common/build.gradle
Outdated
| @@ -22,6 +22,7 @@ test { | |||
| } | |||
|
|
|||
| dependencies { | |||
| compile "io.vertx:vertx-web-api-contract:$vertxVersion" | |||
There was a problem hiding this comment.
do we need this dependency?
There was a problem hiding this comment.
We need either that or io.vertx:vertx-web, changed it to io.vertx:vertx-web to be consistent with main build.gradle
| @@ -144,6 +148,22 @@ public Router vertxRouter(Vertx vertx, LoggerHandler loggerHandler, ErrorHandler | |||
| // Docs index.html page | |||
| StaticHandler docs = StaticHandler.create("docs"); | |||
| router.route().path("/docs/*").handler(docs); | |||
|
|
|||
| // add custom routers | |||
| final String apiVersion = "/api/v1"; | |||
There was a problem hiding this comment.
do we have a constant for this?
| /** | ||
| * Properties maintained in common for all sub projects. | ||
| */ | ||
| public class SidecarProps |
There was a problem hiding this comment.
do we need to introduce this class? Can the property be moved somewhere else?
There was a problem hiding this comment.
Moved that property to FileStreamer
src/main/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandler.java
Outdated
Show resolved
Hide resolved
sarankk
force-pushed
the
move-stream-handler
branch
from
cfa1be9
to
52101ff
Compare
frankgh
approved these changes
Jun 13, 2022
…or streaming file Co-authored-by: Francisco Guerrero <frank.guerrero@gmail.com>
sarankk
force-pushed
the
move-stream-handler
branch
from
baa4ab8
to
37f502a
Compare
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.
Co-authored-by: Francisco Guerrero