Skip to content
This repository has been archived by the owner on Dec 5, 2023. It is now read-only.

add cfn function, checkstyle, cyclic dep #10

Merged
merged 13 commits into from
Mar 14, 2018
Merged

Conversation

geronimo-iia
Copy link
Contributor

used to retrieve cloudformation output value

@geronimo-iia
Copy link
Contributor Author

@flou could you take time to this pull request
thanks

brume/config.py Outdated
@@ -11,6 +11,33 @@
import jinja2


from stack import stack_outputs
# current outputs of loaded stacks
stackOutputsDefinition = {}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use snake_case for variable and methods names.

brume/config.py Outdated
click.secho('[ERROR] No key {} variable in stack {}'.format(key, stack_name), err=True, fg='red')
exit(1)

def cloudformation(stack_name, key, *subKeys):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use snake_case for variables in this function.

brume/config.py Outdated
@@ -29,6 +56,16 @@ def is_git_repo():

class Config():

@staticmethod
def cfn(stack_name, key, secondKey=None, thirdKey=None):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a docstring for this method and update the README with an example usage of the cfn function in brume.yaml?

Also please use snake_case for variables in this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i"ve updated all methods with docstring

brume/stack.py Outdated
outputs = cfn_client().describe_stacks(StackName=stack)['Stacks'][0].get('Outputs', {})
return {o['OutputKey']: o['OutputValue'] for o in outputs}
description = client.describe_stacks(StackName=stack)['Stacks'][0]
stackName = description['StackName']
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use snake_case for variables in this method.

brume/stack.py Outdated
@@ -40,17 +40,32 @@ def _make_parameters(params_list):
return [{'ParameterKey': k, 'ParameterValue': v} for k, v in params_list.items()]


def _outputs_for(stack):
def _stack_walker(client, outputs, stack, collector):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a docstring for this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

brume/config.py Outdated
stackOutputsDefinition = {}

def _check_key_exists(key, container, stack_name):
if not (key in container):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should change this condition to

if key not in container:

brume/config.py Outdated

If `subKey` is specified, return the value of the `subKey` found in the value of the `key` in outputs of specified stack `stack_name`.
"""
if not (stack_name in stackOutputsDefinition):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should change this condition to

if stack_name not in stackOutputsDefinition:

@geronimo-iia
Copy link
Contributor Author

@flou you could see here your requested change,

@geronimo-iia
Copy link
Contributor Author

change are related to #13

@geronimo-iia geronimo-iia changed the title add cfn function add cfn function, checkstyle, cyclic dep Mar 8, 2018
@geronimo-iia
Copy link
Contributor Author

@flou checkstyle are done on most of code,

@geronimo-iia
Copy link
Contributor Author

@flou anytime to check this PR ?

@flou flou merged commit 0944b65 into flou:master Mar 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants