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.

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

Added support to read multiple accounts #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ntkathole
Copy link
Collaborator

<<<<<<< Running the cleanup script in DRY RUN mode >>>>>>>
The AWS provider settings for account aws-account-1 are initialized and validated!
The AWS provider settings for account aws-account-2 are initialized and validated!

Cloudwash execution for account: aws-account-1

Resources from the region: us-east-2

=========== DRY SUMMARY ============


No resources are eligible for cleanup!

====================================


Cloudwash execution for account: aws-account-2

Resources from the region: us-east-1

=========== DRY SUMMARY ============


No resources are eligible for cleanup!

====================================

@jyejare
Copy link
Collaborator

jyejare commented Dec 16, 2022

@ntkathole Holding your PR review for discussion on #79 and doubt if we should really go with multiple conditions and loops. Thats just making the code worst day by day.

I think its the real time for OOPs to introduce and probably we should wait until that for this PR. We still have #79 supporting multiple accounts :)

Any thoughts ?

@liangxiao1
Copy link

This is a common scenario to support clean up resources from different accounts. Especially when run swash as a scheduled service to do clean up. It might be not a good idea to switch account by modifying "~/cloudwash/settings.yaml" again and again.

@jyejare
Copy link
Collaborator

jyejare commented Feb 14, 2023

Hello @liangxiao1 , Thanks for commenting on the PR.

I certainly agree with what you are saying here and also agree with the impact users have on changing the format of settings.yaml instead we could provide an alternate solution to the users for rolling through the multiple users , like creating a settings directory with multiple accounts etc!

Thoughts @ntkathole ?

@liangxiao1
Copy link

Thanks, it is another important feature to specify customized setting files when run cli.
For example, clean up resources from different clouds, like azure and aws. Default "~/cloudwash/settings.yaml" can still be a default cfg file when no another setting file specified.

@ntkathole
Copy link
Collaborator Author

Hello @liangxiao1 , Thanks for commenting on the PR.

I certainly agree with what you are saying here and also agree with the impact users have on changing the format of settings.yaml instead we could provide an alternate solution to the users for rolling through the multiple users , like creating a settings directory with multiple accounts etc!

Thoughts @ntkathole ?

Impact can be handled easily by supporting both old and new settings format and providing deprecation warning to let users know when old settings format is going to be removed. Let me know if you want to support both old and new settings file format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants