You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2025/12/20 12:39:31 UTC
[PR] HDDS-14221. Support primitive query params [ozone]
adoroszlai opened a new pull request, #9537:
URL: https://github.com/apache/ozone/pull/9537
## What changes were proposed in this pull request?
Reuse Ozone configuration's `String` conversion logic to support primitive types for query parameters.
Update `BucketEndpoint#get` to use `getInt`, tests to use `setInt`.
https://issues.apache.org/jira/browse/HDDS-14221
## How was this patch tested?
Ran all S3 unit and integration tests locally.
CI (in progress):
https://github.com/adoroszlai/ozone/actions/runs/20394478143
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #9537:
URL: https://github.com/apache/ozone/pull/9537#issuecomment-3678395791
I'm not sure whether instead of a single long method for handling PUT, GET (e.g. `ObjectEndpoint#put`) we can divide a single endpoint to multiple methods and let the JAX-RS implementation (Jersey) matcher pick the most appropriate ones. For example, for `ObjectEndpoint#put`, we can divide to `putObject`, `putObjectTagging`, `createMultipartKey` based on whether certain headers appear (e.g. ACL header, multipart header). Not sure if there are performance implications.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #9537:
URL: https://github.com/apache/ozone/pull/9537#discussion_r2637207933
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -106,28 +105,28 @@ public class BucketEndpoint extends EndpointBase {
@GET
@SuppressWarnings("methodlength")
public Response get(
- @PathParam(BUCKET) String bucketName,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_KEYS) int maxKeys,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_UPLOADS) int maxUploads
+ @PathParam(BUCKET) String bucketName
) throws OS3Exception, IOException {
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.GET_BUCKET;
PerformanceStringBuilder perf = new PerformanceStringBuilder();
- final String continueToken = getQueryParam(QueryParams.CONTINUATION_TOKEN);
- final String delimiter = getQueryParam(QueryParams.DELIMITER);
- final String encodingType = getQueryParam(QueryParams.ENCODING_TYPE);
- final String marker = getQueryParam(QueryParams.MARKER);
- String prefix = getQueryParam(QueryParams.PREFIX);
- String startAfter = getQueryParam(QueryParams.START_AFTER);
+ final String continueToken = queryParams().get(QueryParams.CONTINUATION_TOKEN);
+ final String delimiter = queryParams().get(QueryParams.DELIMITER);
+ final String encodingType = queryParams().get(QueryParams.ENCODING_TYPE);
+ final String marker = queryParams().get(QueryParams.MARKER);
+ int maxKeys = queryParams().getInt(QueryParams.MAX_KEYS, 1000);
+ final int maxUploads = queryParams().getInt(QueryParams.MAX_UPLOADS, 1000);
Review Comment:
It seems here change to use getInt() not JAX-RS. should we add error hadling in getInt().
now:
https://github.com/apache/ozone/blob/049abeb58891453652a7923d63e3eb988b5454e9/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java#L45-L48
just a suggestion:
```
@Override
public int getInt(String key, int defaultValue) {
String value = get(key);
if (value == null) {
return defaultValue;
}
try {
return Integer.parseInt(value.trim());
} catch (NumberFormatException e) {
throw new OS3Exception(S3ErrorTable.INVALID_ARGUMENT,
key + " must be a valid integer");
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9537:
URL: https://github.com/apache/ozone/pull/9537#issuecomment-3680053644
@adoroszlai , @ivandika3 thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #9537:
URL: https://github.com/apache/ozone/pull/9537#discussion_r2637510543
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -106,28 +105,28 @@ public class BucketEndpoint extends EndpointBase {
@GET
@SuppressWarnings("methodlength")
public Response get(
- @PathParam(BUCKET) String bucketName,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_KEYS) int maxKeys,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_UPLOADS) int maxUploads
+ @PathParam(BUCKET) String bucketName
) throws OS3Exception, IOException {
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.GET_BUCKET;
PerformanceStringBuilder perf = new PerformanceStringBuilder();
- final String continueToken = getQueryParam(QueryParams.CONTINUATION_TOKEN);
- final String delimiter = getQueryParam(QueryParams.DELIMITER);
- final String encodingType = getQueryParam(QueryParams.ENCODING_TYPE);
- final String marker = getQueryParam(QueryParams.MARKER);
- String prefix = getQueryParam(QueryParams.PREFIX);
- String startAfter = getQueryParam(QueryParams.START_AFTER);
+ final String continueToken = queryParams().get(QueryParams.CONTINUATION_TOKEN);
+ final String delimiter = queryParams().get(QueryParams.DELIMITER);
+ final String encodingType = queryParams().get(QueryParams.ENCODING_TYPE);
+ final String marker = queryParams().get(QueryParams.MARKER);
+ int maxKeys = queryParams().getInt(QueryParams.MAX_KEYS, 1000);
+ final int maxUploads = queryParams().getInt(QueryParams.MAX_UPLOADS, 1000);
Review Comment:
sure! I think it makes sense to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9537:
URL: https://github.com/apache/ozone/pull/9537#issuecomment-3677946482
@adoroszlai Thanks for the patch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 merged PR #9537:
URL: https://github.com/apache/ozone/pull/9537
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9537:
URL: https://github.com/apache/ozone/pull/9537#discussion_r2637354520
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -106,28 +105,28 @@ public class BucketEndpoint extends EndpointBase {
@GET
@SuppressWarnings("methodlength")
public Response get(
- @PathParam(BUCKET) String bucketName,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_KEYS) int maxKeys,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_UPLOADS) int maxUploads
+ @PathParam(BUCKET) String bucketName
) throws OS3Exception, IOException {
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.GET_BUCKET;
PerformanceStringBuilder perf = new PerformanceStringBuilder();
- final String continueToken = getQueryParam(QueryParams.CONTINUATION_TOKEN);
- final String delimiter = getQueryParam(QueryParams.DELIMITER);
- final String encodingType = getQueryParam(QueryParams.ENCODING_TYPE);
- final String marker = getQueryParam(QueryParams.MARKER);
- String prefix = getQueryParam(QueryParams.PREFIX);
- String startAfter = getQueryParam(QueryParams.START_AFTER);
+ final String continueToken = queryParams().get(QueryParams.CONTINUATION_TOKEN);
+ final String delimiter = queryParams().get(QueryParams.DELIMITER);
+ final String encodingType = queryParams().get(QueryParams.ENCODING_TYPE);
+ final String marker = queryParams().get(QueryParams.MARKER);
+ int maxKeys = queryParams().getInt(QueryParams.MAX_KEYS, 1000);
+ final int maxUploads = queryParams().getInt(QueryParams.MAX_UPLOADS, 1000);
Review Comment:
We can translate to `WebApplicationException(400)`, but doing so for every type seems cumbersome. Then it may be better to create a new converter just for params, initially supporting only `getInt`/`setInt`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9537:
URL: https://github.com/apache/ozone/pull/9537#issuecomment-3678529311
Thanks @ivandika3, @rich7420 for the review. I have updated the patch to translate to `WebApplicationException(400)`, but stopped using `ConfigurationSource`, has too much unnecessary baggage for this.
> I'm wondering whether instead of a single long method for handling PUT, GET (e.g. `ObjectEndpoint#put`) we can divide a single endpoint to multiple methods and let the JAX-RS implementation (Jersey) matcher router the most appropriate ones ... For example, for `ObjectEndpoint#put`, we can divide to `putObject`, `putObjectTagging`, `createMultipartKey` based on whether certain headers appear (e.g. ACL header, multipart header).
We do have two handlers for `POST` based on content type (`@Consumes`). I'm not very familiar with Jersey, but looking into the docs I haven't found how to do the same based on headers or query params. Would be great if it's possible.
Assuming we have to roll our own, I'd like to:
- define handler for each request as you describe, but in separate objects
- reduce the current long methods into a loop to delegate to the handlers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9537:
URL: https://github.com/apache/ozone/pull/9537#issuecomment-3678633594
@adoroszlai thanks for the update! LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9537:
URL: https://github.com/apache/ozone/pull/9537#discussion_r2637321704
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -106,28 +105,28 @@ public class BucketEndpoint extends EndpointBase {
@GET
@SuppressWarnings("methodlength")
public Response get(
- @PathParam(BUCKET) String bucketName,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_KEYS) int maxKeys,
- @DefaultValue("1000") @QueryParam(QueryParams.MAX_UPLOADS) int maxUploads
+ @PathParam(BUCKET) String bucketName
) throws OS3Exception, IOException {
long startNanos = Time.monotonicNowNanos();
S3GAction s3GAction = S3GAction.GET_BUCKET;
PerformanceStringBuilder perf = new PerformanceStringBuilder();
- final String continueToken = getQueryParam(QueryParams.CONTINUATION_TOKEN);
- final String delimiter = getQueryParam(QueryParams.DELIMITER);
- final String encodingType = getQueryParam(QueryParams.ENCODING_TYPE);
- final String marker = getQueryParam(QueryParams.MARKER);
- String prefix = getQueryParam(QueryParams.PREFIX);
- String startAfter = getQueryParam(QueryParams.START_AFTER);
+ final String continueToken = queryParams().get(QueryParams.CONTINUATION_TOKEN);
+ final String delimiter = queryParams().get(QueryParams.DELIMITER);
+ final String encodingType = queryParams().get(QueryParams.ENCODING_TYPE);
+ final String marker = queryParams().get(QueryParams.MARKER);
+ int maxKeys = queryParams().getInt(QueryParams.MAX_KEYS, 1000);
+ final int maxUploads = queryParams().getInt(QueryParams.MAX_UPLOADS, 1000);
Review Comment:
`OS3Exception` is a checked exception, needs to be declared, but the interface does not allow that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #9537:
URL: https://github.com/apache/ozone/pull/9537#issuecomment-3678861173
Thanks @adoroszlai for the patch and @rich7420 for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9537:
URL: https://github.com/apache/ozone/pull/9537#issuecomment-3682628863
Thanks @ivandika3, @rich7420 for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14221. Support primitive query params [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #9537:
URL: https://github.com/apache/ozone/pull/9537#discussion_r2637501509
##########
hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationTarget.java:
##########
@@ -63,4 +63,8 @@ default <T> void setFromObject(T object) {
ConfigurationReflectionUtil.updateConfiguration(this, object);
}
+ default void unset(String key) {
+ set(key, null);
+ }
Review Comment:
Is this planned to be used in the future patches? Does not seem to be used in this patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org