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.

Sign up for GitHub

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

Jump to bottom

fix terrafrom config bug #14362

Merged
merged 3 commits into from
Jul 14, 2023
Merged

fix terrafrom config bug #14362

merged 3 commits into from
Jul 14, 2023

Conversation

zhongjiajie
Copy link
Member

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2023 •
edited
Loading

Codecov Report

Merging #14362 (199e974) into dev (484f841) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 199e974 differs from pull request most recent head 352cae6. Consider uploading reports for the commit 352cae6 to get more accurate results

@@             Coverage Diff              @@
##                dev   #14362      +/-   ##
============================================
- Coverage     38.73%   38.72%   -0.01%     
+ Complexity     4579     4578       -1     
============================================
  Files          1237     1237              
  Lines         43537    43537              
  Branches       4823     4823              
============================================
- Hits          16863    16859       -4     
- Misses        24804    24807       +3     
- Partials       1870     1871       +1

see 2 files with indirect coverage changes

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

@zhongjiajie zhongjiajie self-assigned this Jun 20, 2023
@Radeity Radeity self-requested a review July 11, 2023 14:56
Radeity
Radeity previously approved these changes Jul 13, 2023
View reviewed changes
Copy link
Member

@Radeity Radeity left a comment

Choose a reason for hiding this comment

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

Others LGTM.

deploy/terraform/aws/README.md
@@ -7,7 +7,7 @@

Set necessary variables by creating a file `ds-ami.pkrvars.hcl` and adding the following variables according to your own usage.

```hcl
```shel
Copy link
Member

Choose a reason for hiding this comment

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

Why not keep hcl?

Copy link
Member Author

Choose a reason for hiding this comment

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

keep hcl is ok to me, but we execute this command into the shell instead of terrafrom. so I change to shell.

Copy link
Member Author

Choose a reason for hiding this comment

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

so, do you prefer to use hcl or shell, all look go for me

Copy link
Member

Choose a reason for hiding this comment

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

Oh, both is okay on me, either.

@zhongjiajie zhongjiajie added this to the 3.2.0 milestone Jul 13, 2023
@zhongjiajie zhongjiajie added bug Something isn't working 3.2.0 for 3.2.0 version labels Jul 13, 2023
@zhongjiajie zhongjiajie marked this pull request as ready for review July 13, 2023 09:40
@sonarcloud
Copy link

sonarcloud bot commented Jul 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@zhongjiajie zhongjiajie requested a review from Radeity July 13, 2023 10:18
@zhongjiajie zhongjiajie merged commit b5d88f2 into apache:dev Jul 14, 2023
@zhongjiajie zhongjiajie deleted the terrafrom-bugfix branch July 14, 2023 01:52
zhongjiajie added a commit that referenced this pull request Jul 20, 2023
@zhongjiajie
fix terrafrom config bug (#14362)
946d173 
(cherry picked from commit b5d88f2)
biaoma-ty pushed a commit to Kasma-Inc/dolphinscheduler that referenced this pull request Aug 17, 2023
@zhongjiajie @biaoma-ty
fix terrafrom config bug (apache#14362)
ba4aa6d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Radeity Radeity Radeity approved these changes

@caishunfeng caishunfeng Awaiting requested review from caishunfeng

Assignees

@zhongjiajie zhongjiajie

Labels
3.2.0 for 3.2.0 version bug Something isn't working ready-to-merge
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@zhongjiajie @codecov-commenter @Radeity