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

Update lambda.py #42106

Open
wants to merge 10 commits into
base: devel
from

Conversation

Projects
None yet
6 participants
@ToROxI

ToROxI commented Jun 29, 2018

Update for module's logic to don't destroy pre-configured Lambda VPC settings if vpc_subnet_ids and vpc_security_group_ids not set. Explicit declaration of destruction instead.

SUMMARY
ISSUE TYPE
  • Feature Pull Request
  • New Module Pull Request
  • Bugfix Pull Request
  • Docs Pull Request
COMPONENT NAME
ANSIBLE VERSION

ADDITIONAL INFORMATION

Update lambda.py
Update for module's logic to don't destroy pre-configured Lambda VPC settings if vpc_subnet_ids and vpc_security_group_ids not set. Explicit declaration of destruction instead.
@ansibot

This comment has been minimized.

Contributor

ansibot commented Jun 29, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jun 29, 2018

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/lambda.py:84:115: W291 trailing whitespace
lib/ansible/modules/cloud/amazon/lambda.py:89:78: W291 trailing whitespace
lib/ansible/modules/cloud/amazon/lambda.py:90:62: W291 trailing whitespace

click here for bot help

Update lambda.py
fix trailing whitespace
@resmo

This comment has been minimized.

Member

resmo commented Jul 1, 2018

I am -1 with the implementation, having a keywork 'None' is a no go. I even wonder, if the functionality is necessary as it can possible be achieved by using an empty list

# Explicit declaration of no-VPC configuration
- name: looped creation
  lambda:
    name: '{{ item.name }}'
    state: present
    zip_file: '{{ item.zip_file }}'
    runtime: 'python2.7'
    role: 'arn:aws:iam::987654321012:role/lambda_basic_execution'
    handler: 'hello_python.my_handler'
    vpc_subnet_ids: []
    vpc_security_group_ids: []
    environment_variables: '{{ item.env_vars }}'
    tags:
      key1: 'value1'
  with_items:
    - name: HelloWorld
      zip_file: hello-code.zip
      env_vars:
        key1: "first"
        key2: "second"
    - name: ByeBye
      zip_file: bye-code.zip
      env_vars:
        key1: "1"
        key2: "2"

needs_info

@ansibot ansibot added needs_info and removed community_review labels Jul 1, 2018

@ToROxI

This comment has been minimized.

ToROxI commented Jul 1, 2018

@resmo,
Hi!
Main goal was to avoid destruction of VPC configuration if parameters are not provided in module.
So if keyword 'None' is not applicable for you - I'll remove this.

Please let me know if you have any concerns about my patch.
Thanks!

@ansibot ansibot added community_review and removed needs_info labels Jul 1, 2018

@s-hertel

Right now both omitting the options from the playbook and setting the options to an empty list remove the current VPC configuration. Rather than setting a special 'None' keyword, it would be better to change that both omitting and setting to [] mean the same thing.
e.g.

  • if vpc_security_group_ids and vpc_subnet_ids are both lists that contain things, set the VPC configuration
  • if they are empty lists, remove the VPC configuration
  • if they are None (module.params['vpc_subnet_ids'] == None when the option isn't provided in the task), leave the VPC configuration as it is
@ansibot

This comment has been minimized.

Contributor

ansibot commented Jul 4, 2018

@ToROxI This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jul 4, 2018

@ToROxI this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

Update lambda.py
Apply requested changes
@ToROxI

This comment has been minimized.

ToROxI commented Jul 4, 2018

@s-hertel, requested change applied

ToROxI added some commits Jul 7, 2018

Update lambda.py
Update for logic of module not to force user to set runtime, handler and role if function already exists.

@s-hertel, please review this change.
Please let me know if you have any concerns about my patch.
Thanks!
Update lambda.py
fix trailing whitespace
@ansibot

This comment has been minimized.

Contributor

ansibot commented Jul 7, 2018

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/lambda.py:84:115: W291 trailing whitespace
lib/ansible/modules/cloud/amazon/lambda.py:89:78: W291 trailing whitespace
lib/ansible/modules/cloud/amazon/lambda.py:90:62: W291 trailing whitespace

click here for bot help

ToROxI added some commits Jul 7, 2018

Update lambda.py
fix logic statement
Update lambda.py
fix logic issue
Update lambda.py
fix logic issue 2
Update lambda.py
change required_together
@ToROxI

This comment has been minimized.

ToROxI commented Jul 7, 2018

@s-hertel , @achinthagunasekara
there is need to update tests due to changes in the invocation sequence. Will you review test or I need to add this changes of test to PR?

Thanks!

Update lambda.py
fix check for unsupported mutation operation
@mattclay

This comment has been minimized.

Member

mattclay commented Jul 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment