Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oss avoid listBuckets permission in bucket check #14414

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

zhangkuantian
Copy link
Contributor

@zhangkuantian zhangkuantian commented Jun 27, 2023

fix #14413

@zhongjiajie
Copy link
Member

approval ci run

zhongjiajie
zhongjiajie previously approved these changes Jun 28, 2023
Copy link
Member

@zhongjiajie zhongjiajie left a comment

Choose a reason for hiding this comment

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

LGTM overall, do you have time to double check @rickchengx

@zhongjiajie zhongjiajie changed the title optimize oss bucket check oss avoid listBuckets permission in bucket check Jun 28, 2023
@zhongjiajie
Copy link
Member

hi @zhangkuantian can you run mvn spotless:apply format your code to pass CI?

@zhongjiajie zhongjiajie added the 3.2.0 for 3.2.0 version label Jun 28, 2023
@zhangkuantian
Copy link
Contributor Author

hi, @zhongjiajie I have already run mvn spotless:apply to format the code. Could you please help me review it?

@codecov-commenter
Copy link

Codecov Report

Merging #14414 (b129643) into dev (5250b25) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head b129643 differs from pull request most recent head 20171aa. Consider uploading reports for the commit 20171aa to get more accurate results

@@             Coverage Diff              @@
##                dev   #14414      +/-   ##
============================================
- Coverage     38.52%   38.51%   -0.01%     
  Complexity     4556     4556              
============================================
  Files          1237     1237              
  Lines         43507    43499       -8     
  Branches       4813     4815       +2     
============================================
- Hits          16761    16755       -6     
- Misses        24889    24890       +1     
+ Partials       1857     1854       -3     
Impacted Files Coverage Δ
...heduler/common/log/remote/OssRemoteLogHandler.java 0.00% <0.00%> (ø)
...heduler/plugin/storage/oss/OssStorageOperator.java 35.74% <0.00%> (+0.56%) ⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sonarcloud
Copy link

sonarcloud bot commented Jun 28, 2023

Please retry analysis of this Pull-Request directly on SonarCloud.

@zhongjiajie zhongjiajie added the bug Something isn't working label Jun 28, 2023
@zhongjiajie
Copy link
Member

we will wait for @rickchengx to see whether he have an addition option here

@zhongjiajie zhongjiajie merged commit 6f90156 into apache:dev Jun 28, 2023
@zhongjiajie
Copy link
Member

Hi @zhangkuantian, welcome to join contributors community 🎉 .
If you want to continue your contribute but could not find issues, maybe you could start in https://github.com/apache/dolphinscheduler/contribute
or just search our issue list https://github.com/apache/dolphinscheduler/issues, Looking for some thing you interesting.

IT-Kwj pushed a commit to IT-Kwj/dolphinscheduler that referenced this pull request Jul 14, 2023
zhongjiajie pushed a commit that referenced this pull request Jul 20, 2023
---------

Co-authored-by: kuantian.zhang <[email protected]>
(cherry picked from commit 6f90156)
biaoma-ty pushed a commit to Kasma-Inc/dolphinscheduler that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.2.0 for 3.2.0 version backend bug Something isn't working first time contributor First-time contributor ready-to-merge
Projects
None yet
4 participants