chore(cli): add svc status output for Static Site service type
#4985
Conversation
|
🍕 Here are the new binary sizes!
|
internal/pkg/aws/s3/s3.go
Outdated
| @@ -251,6 +251,40 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { | |||
| return tree.String(), nil | |||
| } | |||
|
|
|||
| // GetBucketSizeAndCount returns the total size and number of objects in an S3 bucket. | |||
| func (s *S3) GetBucketSizeAndCount(bucket string) (string, int, error) { | |||
There was a problem hiding this comment.
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 have
func (*S3) ListObjects(bucket string) (Objects, error)
type Objects []*s3.Object
func (Objects) TotalObjectCount() int
func (Objects) TotalSize() string
func (Objects) Tree() (string, error)There was a problem hiding this comment.
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.
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).
Codecov Report
@@ Coverage Diff @@
## mainline #4985 +/- ##
============================================
- Coverage 70.16% 70.15% -0.01%
============================================
Files 289 289
Lines 41797 41902 +105
Branches 285 285
============================================
+ Hits 29326 29397 +71
- Misses 11057 11089 +32
- Partials 1414 1416 +2
|
Co-authored-by: Danny Randall <[email protected]>
internal/pkg/cli/svc_status.go
Outdated
| 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.
Maybe consider using a switch here?
dannyrandall
added
the
do-not-merge
huanjani
removed
the
do-not-merge
Very basic output:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.