-
Notifications
You must be signed in to change notification settings - Fork 424
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
chore(cli): add svc status
output for Static Site service type
#4985
Merged
Merged
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
eab904e
chore: add svc status for static sites
huanjani f504e8c
chore: add unit tests
huanjani 55d7a49
chore: add unit test for empty bucket
huanjani bfc28c4
chore: separate out listobjects functionality
huanjani 5288c49
Update internal/pkg/describe/status_describe.go
huanjani f63de23
chore: addr dnrnd fb
huanjani acb496b
chore: return err
huanjani 809ba59
chore: addr dnrnd fb
huanjani 75af207
chore: addr ph fb
huanjani 607feec
chore: tweak unit test error wording
huanjani cc799f0
Merge branch 'mainline' into status-size-number
mergify[bot] 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
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 |
---|---|---|
|
@@ -74,7 +74,18 @@ func newSvcStatusOpts(vars svcStatusVars) (*svcStatusOpts, error) { | |
ConfigStore: configStore, | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("creating status describer for apprunner service %s in application %s: %w", o.svcName, o.appName, err) | ||
return fmt.Errorf("create status describer for App Runner service %s in application %s: %w", o.svcName, o.appName, err) | ||
} | ||
o.statusDescriber = d | ||
} else if wkld.Type == manifestinfo.StaticSiteType { | ||
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. Maybe consider using a switch here? |
||
d, err := describe.NewStaticSiteStatusDescriber(&describe.NewServiceStatusConfig{ | ||
App: o.appName, | ||
Env: o.envName, | ||
Svc: o.svcName, | ||
ConfigStore: configStore, | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("create status describer for Static Site service %s in application %s: %w", o.svcName, o.appName, err) | ||
} | ||
o.statusDescriber = d | ||
} else { | ||
|
@@ -85,7 +96,7 @@ func newSvcStatusOpts(vars svcStatusVars) (*svcStatusOpts, error) { | |
ConfigStore: configStore, | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("creating status describer for service %s in application %s: %w", o.svcName, o.appName, err) | ||
return fmt.Errorf("create status describer for service %s in application %s: %w", o.svcName, o.appName, err) | ||
} | ||
o.statusDescriber = d | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
Oops, something went wrong.
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.
The fact that this public function is specifically designed for status use case makes me think maybe we could rename this function. Otherwise it'll be too coupled with each other and not generic enough.
Also by looking at
GetBucketTree
I realized they share many very similar logics with each other (aka listing objects within the bucket). How about we just haveThere 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.
Hm, yeah. I hadn't reused
GetBucketTree
because of the delimiter difference, but I could pass that in. Will refactor!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 kept total count and total size together (for now) to avoid making another API call.
I have
ListObjects
returning the output because Objects are just one element of the response that we need (we also need prefixes for tree-making).