-
Notifications
You must be signed in to change notification settings - Fork 192
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
[aztfy enhancement proposal] -o
option for specified output dir
#15
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
0ab6c38
Add option for specify output dir
koudaiii 77f1d6b
Terraform should use cache.
koudaiii 979aa08
refactor
koudaiii 8251763
refactor
koudaiii 03fcd83
Update internal/config/config.go
koudaiii cb216e9
output dir corresponds to a resource group
koudaiii 3d9e358
Check that is empty. If not, we shall error out becasuse of we don't…
koudaiii 1cf9525
Update main.go
koudaiii 8ace0cc
ensure `odir` is an empty dir
magodo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import ( | |
"context" | ||
"encoding/json" | ||
"fmt" | ||
"io" | ||
"os" | ||
"path" | ||
"path/filepath" | ||
|
@@ -32,7 +33,7 @@ type MetaImpl struct { | |
armTemplate armtemplate.Template | ||
} | ||
|
||
func newMetaImpl(rg string) (Meta, error) { | ||
func newMetaImpl(rg string, outputDir string) (Meta, error) { | ||
ctx := context.TODO() | ||
|
||
// Initialize the workspace | ||
|
@@ -53,11 +54,35 @@ func newMetaImpl(rg string) (Meta, error) { | |
} | ||
|
||
wsp := filepath.Join(rootDir, rg) | ||
if err := os.RemoveAll(wsp); err != nil { | ||
return nil, fmt.Errorf("removing existing workspace %q: %w", wsp, err) | ||
} | ||
if err := os.MkdirAll(wsp, 0755); err != nil { | ||
return nil, fmt.Errorf("creating workspace %q: %w", wsp, err) | ||
|
||
if outputDir != "" { | ||
wsp = outputDir | ||
|
||
// Ensure wsp is an empty directory | ||
stat, err := os.Stat(wsp) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
if os.IsNotExist(err) { | ||
return nil, fmt.Errorf("the output directory %q doesn't exist", wsp) | ||
} | ||
if !stat.IsDir() { | ||
return nil, fmt.Errorf("the output path %q is not a directory", wsp) | ||
} | ||
|
||
f, err := os.Open(wsp) | ||
if err != nil { | ||
return nil, err | ||
} | ||
_, err = f.Readdirnames(1) // Or f.Readdir(1) | ||
f.Close() | ||
if err != io.EOF { | ||
return nil, fmt.Errorf("the output directory %q is not empty", wsp) | ||
} | ||
} else { | ||
if err := os.RemoveAll(wsp); err != nil { | ||
return nil, fmt.Errorf("removing existing workspace %q: %w", wsp, err) | ||
} | ||
if err := os.MkdirAll(wsp, 0755); err != nil { | ||
return nil, fmt.Errorf("creating workspace %q: %w", wsp, err) | ||
} | ||
} | ||
|
||
// Authentication | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we change the semantic of
outputDir
here a bit to make it the directory containing all the generated stuff, rather than a directory containing sub-directories where each of them corresponds to a resource group? If so, we need further ensure theoutputDir
is an empty directory before removing everything in there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. This makes more sense. I'll try.
I want to clarify, do you mean
outputDir
=wsp
(ref. https://github.com/magodo/aztfy/pull/15/files/8251763c0bbad7b1296acc2c448ff72c3caee413#diff-9f2fd5dbedcbf3f95aa9c36429e20b2b5ba3b63bb777338f8d0c424b8d41939bL55) ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix magodo@cb216e9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@koudaiii Yep! Shall we further check wheter the
wsp
here is empty? If not, we shall error out, so that we don't accidentally remove anything for the user?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review. Ok, We check that
wsp
is empty. If not, we shall error out because we don't accidentally remove anything.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix magodo@3d9e358