-
Notifications
You must be signed in to change notification settings - Fork 422
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 S3 bucket object tree to svc show
#4966
Conversation
🍕 Here are the new binary sizes!
|
Codecov Report
@@ Coverage Diff @@
## mainline #4966 +/- ##
==========================================
Coverage 70.15% 70.15%
==========================================
Files 289 289
Lines 41693 41797 +104
Branches 285 285
==========================================
+ Hits 29248 29324 +76
- Misses 11038 11060 +22
- Partials 1407 1413 +6
|
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.
LGTM overall especially the example included in the description!
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.
LGTM overall especially the example included in the description!
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.
Awesome!
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.
Looks great!! I just have two more questions 🥺 mostly the env manager role session and whether we should let the users know that there is no file in an env instead of not showing the env at all!
listResp := &s3.ListObjectsV2Output{} | ||
var err error | ||
for _, prefix := range prefixes { | ||
var respContents []*s3.Object |
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.
looks like we can look to parallelize these calls in the future!
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.
Looks great! Approving with a label but feel free to remove it whenever you're raedy!
…ecs-cli-v2 into add-bucket-tree
sample output:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.