You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "turboFei (via GitHub)" <gi...@apache.org> on 2024/07/11 04:40:43 UTC
[PR] fix form mediaType [celeborn]
turboFei opened a new pull request, #2616:
URL: https://github.com/apache/celeborn/pull/2616
<!--
Thanks for sending a pull request! Here are some tips for you:
- Make sure the PR title start w/ a JIRA ticket, e.g. '[CELEBORN-XXXX] Your PR title ...'.
- Be sure to keep the PR description updated to reflect all changes.
- Please write your PR title to summarize what this PR proposes.
- If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1674259051
##########
docs/migration.md:
##########
@@ -21,6 +21,14 @@ license: |
# Migration Guide
+## Upgrading from 0.5.0 to 0.5.1
+
+- Since 0.5.1, Celeborn master REST API `/exclude` uses media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
thanks, addressed.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#issuecomment-2223334362
create CELEBORN-1503 for this PR.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type for form urlencoded APIs [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1673556305
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
IMO, you should move this description of migration to `## Upgrading from 0.5 to 0.6` or `## Upgrading from 0.5.0 to 0.5.1`.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#issuecomment-2223302381
just noticed that CELEBORN-1317 was released in 0.5.0, for the released ticket, please create a new one.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#issuecomment-2222022453
cc @SteNicholas @RexXiong @pan3793
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#issuecomment-2223150701
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1673576552
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
with text/plain media type, the form params can not be transferred to server side and were always empty.
so, this change was from 0.5.0
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1674241089
##########
docs/migration.md:
##########
@@ -21,6 +21,14 @@ license: |
# Migration Guide
+## Upgrading from 0.5.0 to 0.5.1
+
+- Since 0.5.1, Celeborn master REST API `/exclude` uses media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
request or response?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1673580367
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
updated the pr title and add `annotations` keyword.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1673568752
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
currently, with text/plain， the request will fail.
This pr does not introduce any user face change.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1673965185
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
@turboFei, although the change was from 0.5.0, this followup would be released in 0.5.1 or 0.6.0. Therefore, you should not update the migration between 0.4 and 0.5.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#issuecomment-2222109618
unrelated test failure
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1674008805
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
ok, updated
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#issuecomment-2223300996
Merged to main/0.5.1
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type for form urlencoded APIs [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1673568752
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
currently, with text/plain， the request will fail.
This pr does not introduce any user face change.
Just fix the media type prompt.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #2616:
URL: https://github.com/apache/celeborn/pull/2616#discussion_r1673866487
##########
docs/migration.md:
##########
@@ -40,9 +40,11 @@ license: |
- Since 0.5.0, Celeborn deprecate `celeborn.client.spark.shuffle.forceFallback.enabled`. Please use `celeborn.client.spark.shuffle.fallback.policy` instead.
-- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead.
+- Since 0.5.0, Celeborn master REST API `/exclude` uses POST method and the parameters `add` and `remove` use form parameters instead, and use media type `application/x-www-form-urlencoded` instead of `text/plain`.
Review Comment:
0.5.0 is already released..
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #2616: [CELEBORN-1317][FOLLOWUP] Fix media type annotations for form urlencoded APIs
URL: https://github.com/apache/celeborn/pull/2616
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org