You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2023/10/10 04:55:04 UTC
[PR] [CELEBORN-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
beliefer opened a new pull request, #1968:
URL: https://github.com/apache/incubator-celeborn/pull/1968
### What changes were proposed in this pull request?
Avoid magic strings copy from Spark `SQLConf`.
### Why are the changes needed?
Currently, the spark integration uses many magic strings copy from Spark `SQLConf`.
Since we have already depend on Spark, references the variable from Spark is better.
### Does this PR introduce _any_ user-facing change?
No.
Just update the inner implementation.
### How was this patch tested?
Exists test cases.
--
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-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture commented on PR #1968:
URL: https://github.com/apache/incubator-celeborn/pull/1968#issuecomment-1754859975
Hi @beliefer , I've reformatted the style, but there is another problem. Since some of the spark configs are introduced in higher spark version, seems we should not change `SparkShuffleManagerSuite`, ``SparkShuffleManagerSuite`, `SkewJoinSuite` and `SparkTestBase`.
--
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-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #1968:
URL: https://github.com/apache/incubator-celeborn/pull/1968#issuecomment-1754397245
please run `./dev/reformat` to automatically reformat code style.
--
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-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #1968:
URL: https://github.com/apache/incubator-celeborn/pull/1968#issuecomment-1755363097
@waitinfuture @cfmcgrady Thank you!
--
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-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #1968:
URL: https://github.com/apache/incubator-celeborn/pull/1968#issuecomment-1754832728
> please run `./dev/reformat` to automatically reformat code style.
Thank you. `dev/reformat` need some dependencies and download them very slowly.
--
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-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture closed pull request #1968: [CELEBORN-1019] Avoid magic strings copy from Spark SQLConf
URL: https://github.com/apache/incubator-celeborn/pull/1968
--
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-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #1968:
URL: https://github.com/apache/incubator-celeborn/pull/1968#issuecomment-1754393355
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1968?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#1968](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1968?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (f058eeb) into [main](https://app.codecov.io/gh/apache/incubator-celeborn/commit/785a035eca94d55b8694c2f607efdd78cb33c417?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (785a035) will **increase** coverage by `0.47%`.
> Report is 7 commits behind head on main.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #1968 +/- ##
==========================================
+ Coverage 46.51% 46.97% +0.47%
==========================================
Files 164 164
Lines 10341 10362 +21
Branches 954 957 +3
==========================================
+ Hits 4809 4867 +58
+ Misses 5218 5178 -40
- Partials 314 317 +3
```
[see 12 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1968/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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-1019] Avoid magic strings copy from Spark SQLConf [incubator-celeborn]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #1968:
URL: https://github.com/apache/incubator-celeborn/pull/1968#issuecomment-1754894955
> Hi @beliefer , I've reformatted the style, but there is another problem. Since some of the spark configs are introduced in higher spark version, seems we should not change `CelebornShuffleManagerSuite` in spark-2 directory, `SkewJoinSuite` and `SparkTestBase`.
Thank you. I will check them.
--
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