Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Core multiple stacks management #215

Merged
merged 3 commits into from
Aug 10, 2016
Merged

Core multiple stacks management #215

merged 3 commits into from
Aug 10, 2016

Conversation

yufangzhang
Copy link
Contributor

@yufangzhang yufangzhang commented Jul 4, 2016

  • Modify fab task cfn_create, cfn_delete to mange multiple stacks.
  • Create task set_active_stack(tag) to switch among stacks
  • Add unit tests for new methods

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.4%) to 48.066% when pulling aeacbc9 on create_multiple_stacks into 11b3fed on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.4%) to 48.066% when pulling 59d15be on create_multiple_stacks into 11b3fed on master.

"""
r53_conn = get_connection(R53)
zone_id = get_zone_id()
record = "{}".format(get_tag_record_name(stack_tag))
Copy link
Contributor

Choose a reason for hiding this comment

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

This could probably be simply:

record = get_tag_record_name(stack_tag)

i.e. without the call to format.

@sevenmachines
Copy link
Contributor

@yufangzhang happy to merge after those couple of minor things @mattpep mentioned are fixedup!

@yufangzhang
Copy link
Contributor Author

@mattpep I've done the changes. can you have a look ~?

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.3%) to 48.157% when pulling 5136661 on create_multiple_stacks into 11b3fed on master.

@sevenmachines
Copy link
Contributor

@mattpep @yufangzhang we happy to merge?

@mattpep
Copy link
Contributor

mattpep commented Jul 12, 2016

LGTM. Ideally we should squash the fixup commit into the one it updates. Then it's good to merge.

Args:
stack_tag: the tag of stack
Returns:
True if stack exists
Copy link
Contributor

@filipposc5 filipposc5 Jul 12, 2016

Choose a reason for hiding this comment

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

Is this accurate? To be specific does it return "True" if stack exists?

@yufangzhang
Copy link
Contributor Author

@mattpep yeup, updated!

@mattpep
Copy link
Contributor

mattpep commented Jul 13, 2016

The first bullet point of this PR makes reference to changing the task cfn_create but I see no changes of that task within config.py.
Does the stack_id now have to be specified? More generically, it'd be useful for the documentation be updated to explain how these new tasks work.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.4%) to 48.159% when pulling c084386 on create_multiple_stacks into b24ca77 on master.

@yufangzhang
Copy link
Contributor Author

@mattpep we generate stack_id from uuid in set_stack_name at the beginning as what it used to be, then add it to stack_name. In config.py, we only added self.stack_id as instance variable in config and changed the format of dns record name.

@yufangzhang yufangzhang force-pushed the create_multiple_stacks branch 3 times, most recently from 1d167ad to 7d5479e Compare July 26, 2016 13:13
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 7d5479e on create_multiple_stacks into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b3bc543 on create_multiple_stacks into * on master*.

@yufangzhang yufangzhang force-pushed the create_multiple_stacks branch 2 times, most recently from dcd5228 to f34f0f4 Compare July 27, 2016 13:48
@coveralls
Copy link

coveralls commented Jul 27, 2016

Coverage Status

Changes Unknown when pulling f34f0f4 on create_multiple_stacks into * on master*.

@yufangzhang yufangzhang changed the title [WIP] Core multiplestacks operations [WIP] Core multiple stacks management Jul 27, 2016
@yufangzhang yufangzhang changed the title [WIP] Core multiple stacks management Core multiple stacks management Jul 27, 2016
r53_conn = get_connection(R53)
zone_name = cfn_config.data.get('master_zone', None)
record = "{}.{}".format(get_tag_record_name(stack_tag), zone_name)
logger.info("fab_tasks::set_stack_name: "
Copy link

@cliffxuan cliffxuan Aug 4, 2016

Choose a reason for hiding this comment

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

format is better than using %s in formatting a string, however, in logging, %s is actually preferred, so you would do:

logger.info("Creating stack suffix '%s' for record '%s'... %s...", stack_suffix, record, zone_id)

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1df9b52 on create_multiple_stacks into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b88af34 on create_multiple_stacks into * on master*.

@yufangzhang
Copy link
Contributor Author

@cliffxuan Thanks for the advice Cliff! I've made some changes to make it more pythonic 👻
@lukaszraczylo think you may have used this to do the CCCD stuff? I've just updated a bug on deleting active stack records. hopefully you haven't come across that....
@filipposc5 @niallcreech I think this beast is ready to be merged, can anyone have a look? sorry about its size :P

yufangzhang added 3 commits August 10, 2016 16:15
Test starts failing with the error below, which is fixed in 2a595f1
(aka version 0.9 unreleased):
```
Traceback (most recent call last):
  File "/Users/yufang/.virtualenvs/test/bin/flake8", line 11, in <module>
    sys.exit(main())
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/main/cli.py", line 16, in main
    app.run(argv)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/main/application.py", line 316, in run
    self._run(argv)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/main/application.py", line 299, in _run
    self.initialize(argv)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/main/application.py", line 290, in initialize
    self.register_plugin_options()
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/main/application.py", line 150, in register_plugin_options
    self.check_plugins.register_options(self.option_manager)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/plugins/manager.py", line 451, in register_options
    list(self.manager.map(register_and_enable))
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/plugins/manager.py", line 261, in map
    yield func(self.plugins[name], *args, **kwargs)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/plugins/manager.py", line 447, in register_and_enable
    call_register_options(plugin)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/plugins/manager.py", line 357, in generated_function
    return method(optmanager, *args, **kwargs)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8/plugins/manager.py", line 207, in register_options
    add_options(optmanager)
  File "/Users/yufang/.virtualenvs/test/lib/python2.7/site-packages/flake8_import_order/flake8_linter.py", line 32, in add_options
    parser.config_options.append("application-import-names")
AttributeError: 'OptionManager' object has no attribute 'config_options'
```
update cfn_create + cfn_delete with multiple stacks support
add set_active_stack fab task
Add unittest on newly added methods.
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e920fc1 on create_multiple_stacks into * on master*.

@ezman ezman merged commit d917519 into master Aug 10, 2016
@mattpep mattpep deleted the create_multiple_stacks branch August 11, 2016 09:07
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.

None yet

7 participants