Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-18 Thread via GitHub 


ok2c commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2403805629


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:
##
@@ -1334,6 +1359,38 @@ H2Stream createStream(final H2StreamChannel channel, 
final H2StreamHandler strea
 return streams.createActive(channel, streamHandler);
 }
 
+public final void sendPriorityUpdate(final int prioritizedStreamId, final 
PriorityValue value) throws IOException {

Review Comment:
   @arturobernalg Please also remove `H2RequestPriority` interceptor for now. 
It is not being used anywhere in core and I presume your intention was to use 
it in client. In this case the interceptor should be in client.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-18 Thread via GitHub 


arturobernalg commented on PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#issuecomment-3366605376

   > @arturobernalg Please also add RFC 9218 to the protocol support section in 
`README.md`
   
   @ok2c done


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-18 Thread via GitHub 


arturobernalg merged PR #552:
URL: https://github.com/apache/httpcomponents-core/pull/552


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-17 Thread via GitHub 


ok2c commented on PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#issuecomment-3366549940

   @arturobernalg Please also add RFC 9218 to the protocol support section in 
`README.md`


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-17 Thread via GitHub 


ok2c commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2403806904


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:
##
@@ -1334,6 +1359,38 @@ H2Stream createStream(final H2StreamChannel channel, 
final H2StreamHandler strea
 return streams.createActive(channel, streamHandler);
 }
 
+public final void sendPriorityUpdate(final int prioritizedStreamId, final 
PriorityValue value) throws IOException {

Review Comment:
   @arturobernalg Also `PriorityFormatter` and `PriorityParser` are not being 
used anywhere in core. If they are not used in core they should not be in core. 
Re-submit them in client. Believe me you do not want to depend on a new release 
of core to be able to fix a bug in _client_.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-17 Thread via GitHub 


arturobernalg commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2403803417


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:
##
@@ -1334,6 +1359,38 @@ H2Stream createStream(final H2StreamChannel channel, 
final H2StreamHandler strea
 return streams.createActive(channel, streamHandler);
 }
 
+public final void sendPriorityUpdate(final int prioritizedStreamId, final 
PriorityValue value) throws IOException {

Review Comment:
   @ok2c Yes, that was intentional. The logic was inlined into the stream 
submit path, so the standalone helper became redundant. I'm going to removed it.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-05 Thread via GitHub 


ok2c commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2403806904


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:
##
@@ -1334,6 +1359,38 @@ H2Stream createStream(final H2StreamChannel channel, 
final H2StreamHandler strea
 return streams.createActive(channel, streamHandler);
 }
 
+public final void sendPriorityUpdate(final int prioritizedStreamId, final 
PriorityValue value) throws IOException {

Review Comment:
   @arturobernalg Also `PriorityFormatter` is not being used anywhere in core. 
If they are not used in core they should not be in core. Re-submit them in 
client. Believe me you do not want to depend on a new release of core to be 
able to fix a bug in _client_.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-04 Thread via GitHub 


ok2c commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2403824113


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:
##
@@ -1334,6 +1359,38 @@ H2Stream createStream(final H2StreamChannel channel, 
final H2StreamHandler strea
 return streams.createActive(channel, streamHandler);
 }
 
+public final void sendPriorityUpdate(final int prioritizedStreamId, final 
PriorityValue value) throws IOException {

Review Comment:
   @arturobernalg Please let me know if you can live with #567. The 
functionality should be moved to client and we should put more work into the 
API design



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-03 Thread via GitHub 


arturobernalg commented on PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#issuecomment-3367951291

   > @arturobernalg I did a sloppy job reviewing your code. It needs more work.
   
   @ok2c Can you point out the specific areas you find lacking, so I can focus 
the fixes where needed? I’ll take another pass, tighten the code paths,


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-03 Thread via GitHub 


ok2c commented on PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#issuecomment-3366825294

   @arturobernalg I did a sloppy job reviewing your code. It needs more work.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-03 Thread via GitHub 


ok2c commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2402860373


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:
##
@@ -1334,6 +1359,38 @@ H2Stream createStream(final H2StreamChannel channel, 
final H2StreamHandler strea
 return streams.createActive(channel, streamHandler);
 }
 
+public final void sendPriorityUpdate(final int prioritizedStreamId, final 
PriorityValue value) throws IOException {

Review Comment:
   @arturobernalg By the way this method does not seem to be used anywhere. Is 
this intended?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-03 Thread via GitHub 


arturobernalg commented on PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#issuecomment-3366123490

   > @arturobernalg This feature is now your area of responsibility.
   
   @ok2c  Understood. I’ll take ownership of the RFC 9218 prioritization 
feature and handle its maintenance going forward.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-02 Thread via GitHub 


arturobernalg commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2399048133


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/priority/PriorityParamsParser.java:
##
@@ -0,0 +1,136 @@
+/*
+ * 
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * 
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * .
+ *
+ */
+package org.apache.hc.core5.http2.priority;
+
+import java.util.Locale;
+
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.util.CharArrayBuffer;
+import org.apache.hc.core5.util.Tokenizer;
+
+@Internal
+public final class PriorityParamsParser {
+private static final Tokenizer TK = Tokenizer.INSTANCE;
+private static final Tokenizer.Delimiter KEY_DELIMS = 
Tokenizer.delimiters('=', ',', ';');
+private static final Tokenizer.Delimiter VALUE_DELIMS = 
Tokenizer.delimiters(',', ';');
+
+private PriorityParamsParser() {
+}
+
+public static PriorityParams parse(final String headerValue) {
+Integer urgency = null; // null means “u absent”
+Boolean incremental = null; // null means “i absent”
+
+if (headerValue == null || headerValue.isEmpty()) {
+return new PriorityParams(null, null);
+}
+
+final CharArrayBuffer buf = new CharArrayBuffer(headerValue.length());

Review Comment:
   @ok2c  please do another pass



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-02 Thread via GitHub 


ok2c commented on code in PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#discussion_r2398582705


##
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/priority/PriorityParamsParser.java:
##
@@ -0,0 +1,136 @@
+/*
+ * 
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * 
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * .
+ *
+ */
+package org.apache.hc.core5.http2.priority;
+
+import java.util.Locale;
+
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.util.CharArrayBuffer;
+import org.apache.hc.core5.util.Tokenizer;
+
+@Internal
+public final class PriorityParamsParser {
+private static final Tokenizer TK = Tokenizer.INSTANCE;
+private static final Tokenizer.Delimiter KEY_DELIMS = 
Tokenizer.delimiters('=', ',', ';');
+private static final Tokenizer.Delimiter VALUE_DELIMS = 
Tokenizer.delimiters(',', ';');
+
+private PriorityParamsParser() {
+}
+
+public static PriorityParams parse(final String headerValue) {
+Integer urgency = null; // null means “u absent”
+Boolean incremental = null; // null means “i absent”
+
+if (headerValue == null || headerValue.isEmpty()) {
+return new PriorityParams(null, null);
+}
+
+final CharArrayBuffer buf = new CharArrayBuffer(headerValue.length());

Review Comment:
   @arturobernalg Please do not do that. There is no need to create a copy of 
the header value. The parser can work with any `CharSequence`.  Can't you 
re-use `MessageSupport` parsing routines here?
   
   Otherwise, looks good.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] HTTP/2: implement RFC 9218 Extensible Prioritization Scheme for HTTP. [httpcomponents-core]

2025-10-01 Thread via GitHub 


arturobernalg commented on PR #552:
URL: 
https://github.com/apache/httpcomponents-core/pull/552#issuecomment-3355444771

   > @arturobernalg This is a big change. I have no bandwidth to carefully read 
through RFC 9218 at the moment. What I can do to review your changes for impact 
on the existing code and APIs. The rest becomes your area of responsibility, if 
you are willing to maintain this feature. Please also consider how stable it is 
and whether one release cycle should be enough to stabilize it or it should be 
marked experimental and go though several release cycles.
   
   @ok2c  ACK — I’ll can own RFC 9218: client emission gated by 
`SETTINGS_NO_RFC7540_PRIORITIES=1`, server accept-only; please review 
API/surface impact


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]