Skip to content

Conversation

@yaooqinn
Copy link
Member

Why are the changes needed?

Some of the commands were migrated to v2 in spark v3.3, this PR is used to match them

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@yaooqinn
Copy link
Member Author

cc @cfmcgrady @zhaomin1423 @packyan

@codecov-commenter
Copy link

codecov-commenter commented May 17, 2022

Codecov Report

Merging #2675 (a55be80) into master (b40bcbd) will decrease coverage by 0.22%.
The diff coverage is 9.63%.

@@             Coverage Diff              @@
##             master    #2675      +/-   ##
============================================
- Coverage     64.24%   64.02%   -0.23%     
- Complexity       82       84       +2     
============================================
  Files           385      385              
  Lines         18673    18737      +64     
  Branches       2531     2549      +18     
============================================
- Hits          11997    11996       -1     
- Misses         5532     5576      +44     
- Partials       1144     1165      +21     
Impacted Files Coverage Δ
...park/authz/ranger/FilterDataSourceV2Strategy.scala 57.14% <0.00%> (-22.86%) ⬇️
...n/spark/authz/ranger/FilteredShowObjectsExec.scala 42.10% <0.00%> (-37.90%) ⬇️
...k/authz/ranger/RuleReplaceShowObjectCommands.scala 60.00% <0.00%> (-2.07%) ⬇️
...he/kyuubi/plugin/spark/authz/util/AuthZUtils.scala 46.80% <0.00%> (-4.36%) ⬇️
.../kyuubi/plugin/spark/authz/PrivilegesBuilder.scala 68.84% <8.00%> (-11.09%) ⬇️
...ache/kyuubi/plugin/spark/authz/OperationType.scala 57.64% <25.00%> (-12.36%) ⬇️
...rg/apache/kyuubi/engine/trino/TrinoStatement.scala 66.66% <0.00%> (-4.60%) ⬇️
...ache/kyuubi/engine/spark/SparkProcessBuilder.scala 83.87% <0.00%> (-1.62%) ⬇️
.../org/apache/kyuubi/operation/KyuubiOperation.scala 68.00% <0.00%> (-1.34%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b40bcbd...a55be80. Read the comment docs.

@yaooqinn yaooqinn added this to the v1.6.0 milestone May 17, 2022
@yaooqinn yaooqinn self-assigned this May 17, 2022
@yaooqinn yaooqinn requested a review from ulysses-you May 17, 2022 07:09
case "ShowFunctionsCommand" =>
getPlanField[Option[String]]("db").foreach { db =>
inputObjs += databasePrivileges(db)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we need this change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using filtered command instead of this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it is not for compatibility but for clean up

@cfmcgrady
Copy link
Contributor

Since the spark-3.3 support is in progress, I have run a GA job on my forked repo. see cfmcgrady#16

@cfmcgrady
Copy link
Contributor

Since the spark-3.3 support is in progress, I have run a GA job on my forked repo. see cfmcgrady#16

CI passed!
截屏2022-05-17 下午5 16 15

@yaooqinn
Copy link
Member Author

thanks @cfmcgrady for checking

@pan3793
Copy link
Member

pan3793 commented May 18, 2022

Thanks, merging to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants