-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Aws eks ciac 10721 #36689
base: master
Are you sure you want to change the base?
Aws eks ciac 10721 #36689
Conversation
This PR is marked as 'Stale' because it has been open for 30 days with no activity, it will be automatically closed in 15 days if no activity will be done. To reset the counter just remove the 'Stale' label or make changes to update this PR. If you wish this PR will never be marked as 'Stale' add the 'Ignore Stale' |
|
1 similar comment
|
18bc1cb
to
0c2284f
Compare
…o/content into aws-eks-ciac-10721
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.
Great work! please see me comments! and sorry for the delay !
@@ -383,7 +427,7 @@ def update_access_entry_command(aws_client, args: dict) -> CommandResults: | |||
) | |||
|
|||
|
|||
def test_module(aws_client) -> str: | |||
def test_module() -> str: # pragma: no cover |
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 think you can add unit test
@@ -65,6 +98,7 @@ def list_clusters_command(aws_client, args: dict) -> CommandResults: | |||
Returns: | |||
A Command Results object | |||
""" | |||
aws_client = build_client(args) |
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.
why to call this on each command and not create the client in the main and send as param?
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 client needs to be built inside the function to support the @run_on_all_accounts decorator, which requires a separate session for each account.
Packs/AWS-ACM/ReleaseNotes/1_1_41.md
Outdated
|
||
##### AWS - ACM | ||
|
||
- Documentation and metadata improvements. |
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.
im not sure this should be the RN lets think together.
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.
Is it more appropriate now?
Contributing to Cortex XSOAR Content
Make sure to register your contribution by filling the contribution registration form
The Pull Request will be reviewed only after the contribution registration form is filled.
Status
Related Issues
fixes: link to the issue
Description
A few sentences describing the overall goals of the pull request's commits.
Must have