Showing posts with label python. Show all posts
Showing posts with label python. Show all posts

Monday, June 1, 2026

AWS: Cloudshell Python version mismatch

 Lately, I tried to install a Python library onto Cloudshell. I first checked the Python version:

$ python --version

Python 3.13.13

So I ran pip install, which deployed a version of my lib compatible with Python 3.13. The lib declares some entry points for setuptools, so that I can run some commands from the CLI. So I run those commands from the shell, but they fail with some errors about missing import.

After investigation, I found out that the command scripts started with a shebang like this one:

#!/usr/bin/python3

This is all normal, except that if I check the link, this is what I find:

$ ls -l /usr/bin/python3

lrwxrwxrwx. 1 root root 9 Apr 20 22:07 /usr/bin/python3-> python3.9

Someone forgot something? Or am I looking at it wrong?

Tuesday, May 12, 2026

AWS: Duplicates in Search Provisioned Products

 Using our beloved boto3 library, we are looking for the list of all our Provisioned Products in Service Catalog.

sc_client = boto3.client('servicecatalog')
result = sc_client.search_provisioned_products(
    PageSize=20
)

I won't bore you with th code that loops over the result and perform the operation again if we have more than 20 products. But the strange thing, is that wherever we had more than the page size, some products were repeated in the other pages. Thinking of a bug in AWS Service Catalog, we reached out to the Support Team. This is their answer:

This is a known behavior with the SearchProvisionedProducts API when using the default relevance-based sorting. Because results are sorted by relevance, the ordering can shift slightly between paginated requests, which causes duplicates (or occasionally missed items) across pages.

Never heard of relevance-based sorting. Looking at the documentation, there is no mention of it:

SortBy

The sort field. If no value is specified, the results are not sorted. The valid values are arnidname, and lastRecordId.

 Then, Support Team is proposing a solution:

Adding SortBy='createdTime' gives the pagination a stable ordering, so the page token points to a consistent boundary between pages. No more duplicates should appear regardless of how many provisioned products you have.

It is interesting to note that 'createdTime' is not listed in the documentation either. We tried it and it works. So a hidden feature solves a known bug.

Thursday, April 18, 2024

WTF: E-mail Validation

 E-mail validation is usually a hard task, but in our case, we had a simple regular expression that allowed us to accept a list of e-mails in a known format. Here is the regular expression that you could find in our code:

^$|^\s*[\w+.-]+@[a-zA-Z_-]+?\.[a-zA-Z]{2,3}(?:,[\w+.-]+@[a-zA-Z_-]+?\.[a-zA-Z]{2,3})*\s*$

Many things here, let's decompose:

  • ^$|: we accept an empty string
  • ^\s*: we ignore leading white space characters
  • [\w+.-]+: the user name part of the e-mail. We accept all words characters, plus sign, dot and dash. 
  • @: the at sign
  • [a-zA-Z_-]+?: the domain name, which can have any letter, dash and underscore. Note here the use of the +? pattern, which is very strange. I had to google it, it is the lazy expansion, which means take the minimum number of character needed to fulfill the pattern. Completely useless here since we are looking for a dot character afterward.
  • \. The dot character between the domain name and the extension
  • [a-zA-Z]{2,3}: the extension, which can be 2 or 3 letters (like .fr or .com)
  • (?:, ... )*: we repeat here the whole pattern to say that we can have any number of other e-mails separated by a comma. Note the strange use of the ?: pattern. I had to google that one too. This is the non capturing group, which means that it is a group that you can not retrieve later using group() functions. Useless here since we are not checking for capturing groups.
  • \s*$: we ignore all trailing space characters
A bit complicated, but still ok. But then, somebody complained that it is not supporting e-mails from our Japanese branch, which have extensions in the form of @domain.co.jp. So someone was set to the task, and came up with the following regular expression:
^$|^\s*[\w+.-]+@[a-zA-Z_-]+?\.[a-zA-Z]{2,3}(?:,[\w+.-]+@[a-zA-Z_-]+?\.[a-zA-Z]{2,3}\.[a-zA-Z]{2,3})*\s*$

The only difference with the previous one is that there is a new \.[a-zA-Z]{2,3} added within the parenthesis. Which mean that you can have japanese style e-mails, but only after the first e-mail of the list. Worse, you can only have japanese style e-mails from the second mail onward. I notified the person that commited the code, and he said that he will think about the problem. Of course, code went to prod...

So I decided to make a quick fix. I removed all the strange patterns, and set the following regular expression:

^$|^\s*[\w+.-]+@[a-zA-Z_-]+(\.[a-zA-Z]{2,3}){1,2}(,[\w+.-]+@[a-zA-Z_-]+(\.[a-zA-Z]{2,3}){1,2})*\s*$

The fix was made using the {1,2} pattern to say that we can have one or two extensions. Meanwhile, the guy who made the first change also started to make a fix. Small communication problem here, he didn't noticed that I already assigned the bug to myself. But the funny thing is that he had a fix on a branch that was never merged. It looked like this:

^$|^\s*[\w+.-]+@(?:domain)+?(\.[a-zA-Z]{2,3}|\.[a-zA-Z]{2,3}\.[a-zA-Z]{2,3})(?:,[\w+.-]+@(?:domain)+?(\.[a-zA-Z]{2,3}|\.[a-zA-Z]{2,3}\.[a-zA-Z]{2,3}))(?:,[\w+.-]+@(?:domain)+?(\.[a-zA-Z]{2,3}|\.[a-zA-Z]{2,3}\.[a-zA-Z]{2,3}))*\s*$

I don't even want to know if it is correct... 

Friday, March 15, 2024

AWS: Find Root Cause of Failure for CloudFormation Stacks

 When a CloudFormation stack fails, you have to scroll back trough the events to find the root cause of the failure. Recently, AWS even added a "Detect Root Cause" button to the Console to immediately scroll to the correct event. But how do you do it from a python script?

import boto3

def find_root_cause(stack_name):
    cf_client = boto3.client('cloudformation')

    next_values = "First Time"
    params = {
        "StackName": stack_name
    }
    root_cause = None

    while next_values:
        result = cf_client.describe_stack_events(**params)

        next_values = result.get("NextToken")
        params["NextToken"] = next_values

        for event in result["StackEvents"]:
            status = event.get("ResourceStatus", "")
            reason = event.get("ResourceStatusReason")

            # start of deployment
            if reason == "User Initiated":
                return root_cause
           
            if reason and "FAILED" in status:
                root_cause = reason

    return root_cause

You follow the same pattern as from the Console. You go back the events history, until you reach the oldest error message before the start of the deployment.

Wednesday, November 15, 2023

Python: ruamel.yaml lib has a problem handling comments

 In our project, we are using the ruamel.yaml library for handling reading/writing YAML files. The reason we are not using yaml basic lib from Python is that ruamel handles better yaml standard, keeps the comments and formatting, and always dumps the keys in the same order.

However, since version 0.18.3, we had some strange behavior in our file dump. Some newlines were removed from some files. I opened ticket #492, with the following code that replicates the problem:

import ruamel.yaml

y = ruamel.yaml.YAML()
with open("organizational_units.yaml", "r") as file:
    ou = y.load(file)

with open("organizational_units.yaml", "r") as file:
    content = y.load(file)

content["organizational_units"] = ou["organizational_units"]

with open("test.yaml", "w") as file:
    y.dump(content, file)
with open("test.yaml", "w") as file:
    y.dump(content, file)

with open("test.yaml", "r") as file:
    y.load(file)

It is of course an oversimplified version of what we are doing in our project. We are normally loading several YAML files and combine them into one big model. Then, when we need to save changes into one file, we first reload it into memory in order to retrieve the original comments at the beginning of the file before replacing the old content with the new one.

Then you can see that we are saving our file twice. In fact, we are really performing a first save into an in-memory string stream, before logging the content in the file (at least in debug mode). Then we are saving it. Again, this code here is a simplification just to display the problem.

The problem occurs on the second save. The first works fine. Using this file as an example:

# Organizational Unit Specification

organizational_units:

- Name: root
  Accounts:
  - FirstAccount
  - SecondAccount

After the second save, we have this result:

# Organizational Unit Specification

organizational_units: -
  Name: root
  Accounts:
  - FirstAccount
  - SecondAccount

Noticed the missing newlines?

The last line of the code is loading the resulting file, just to show that we can not read it back.

After opening the ticket, I got the answer (on the same day, nice reactivity!) that it is in fact the duplicate of ticket #410. The #410 is a bit different, because it duplicates the complete structure, while we are only replacing a part of it. So maybe that is why our code was still working. I think the part that broke it is coming from this fix: "fix issue with spurious newline on first item after comment + nested block sequence".

As the developer explains, the issue is coming from the way the library is storing comments internally. It seems that comments are stored in different places, with the same reference. And when they are dumped, to avoid saving them several times, there is some internal bookkeeping going on. When we replaced reference to the top key, we broke some comments reference.

As a workaround, I restored comments reference around the top key:

comments = content["organizational_units"].ca.comment
content["organizational_units"] = ou["organizational_units"]
content["organizational_units"].ca.comment = comments

Worked for me...

Wednesday, October 25, 2023

Moto: Alias Issue when Creating S3 Access Point is Fixed

 Two days ago, I discovered a small bug in the moto library that I use to unit test my lambdas on AWS. I needed to create an S3 Access Point with boto3, and while retrieving the its alias, I had different results when using the return value of create_access_point or get_access_point.

So I wrote a small unit test:

from moto import mock_s3control
import boto3

@mock_s3control
def test_access_point_alias():
    client = boto3.client("s3control")

    alias_from_create = client.create_access_point(
        AccountId="123456789012",
        Name="my-access-point",
        Bucket="MyBucket",
    )["Alias"]

    alias_from_get = client.get_access_point(
        AccountId="123456789012",
        Name="my-access-point",
    )["Alias"]

    assert alias_from_create == alias_from_get

I create an S3 Access Point, and retrieve its alias in two ways: from the response of the create_access_point function, and from the get_access_point function. On my moto 4.2.6, this test fails.

So I opened an issue on the project's repository. It was fixed and closed on the same day. That's reactivity!

Thursday, October 12, 2023

AWS: The Next Token Pattern

When developing  for AWS, there is a pattern that you use each time a response to a service may return a lot of data. You get back some of the data, together with a token that you can provide to get another round of data.

As an example, let's take the service that returns the list of events from a Cloudformation template deployment. Here is how you would do it using Python and boto3:

cf_client = boto3.client("cloudformation")
response = cf_client.describe_stack_events(StackName="mystack")
# do something with the response

while response.get("NextToken"):
    response = cf_client.describe_stack_events(
        StackName="mystack",
        NextToken=response.get("NextToken")
    )
    # do again something with the response

However, there is one thing that I do not like with this pattern: code repeat. You call the service at two different parts of your code, with almost identical parameters. And you process the response in the same way, again in two places. If you have to fix something in this code, you have to remember to fix in both places.

The approach I use to have your code only once, is to take advantage of Python's capacity to pass parameters as a dictionary. Here is my approach to this pattern:

cf_client = boto3.client("cloudformation")
next_token = "FIRST TIME"
params = {"StackName": "mystack"}

while next_token:
    response = cf_client.describe_stack_events(**params)
    # do something with the response

    next_token = response.get("NextToken")
    params["NextToken"] = next_token

I store my parameter list in a dictionary, and I initialize the next token with something that is not empty. So the first time in the loop will always run. I can then call my service, without the token. After processing the response, I then read the next token and fill it in my parameter list. The second time round, it will call the service with my token.

Something funny happened when I started using this pattern into our production code. We tried using Bandit, which is a tool that analyze your code and looks for security issues. It would systematically flag my pattern with this error: 

[B105:hardcoded_password_string] Possible hardcoded password: 'FIRST TIME'

 Well, I have to slightly modify my pattern to avoid using the word token...

Wednesday, October 11, 2023

AWS: Simpler S3 File Deletes by Prefix

 I came across this code that deletes files in an S3 from a list of prefixes:

s3_client = boto3.client('s3')
for prefix in prefix_list:
    paginator = s3_client.get_paginator('list_objects_v2')
    file_list = paginator.paginate(
        Bucket=data_bucket,
        Prefix=prefix
    )
    for current_content in file_list:
        for current_file in current_content.get('Contents', []):
            current_key = current_file['Key']
            response = s3_client.delete_object(
                Bucket=data_bucket,
                Key=current_key
            )

The code creates an S3 client, and then, for each prefix in a list, it creates a paginator. Paginators are great because they help you avoid using all your memory when the list of files is big. Using this paginator, the code retrieves the list of all the files corresponding to the prefix, and deletes it.

Nothing bad in the code, it works nicely. My only remark here, is that there exists a simpler way. Instead of using the S3 client, you can create an S3 bucket resource. From there, you can simply delete all files listed under a prefix using a simple filter:

s3_resource = boto3.resource('s3')
bucket = s3_resource.Bucket(data_bucket)
for prefix in prefix_list:
    bucket.objects.filter(Prefix=prefix).delete()

Simpler!

Friday, October 6, 2023

AWS: Automatic Subscription Confirmation from SQS Queue to SNS Topic

 We have an architecture in AWS where different events from different accounts need to be sent to one central SQS queue. Since the events will cross both accounts and regions, one way to do it is to send them to a local SNS Topic. 

The SQS queue will have to subscribe to all those Topics, but we can not do it on the SQS side, since it does not know each time someone pops out a new account. However, the problem with having the SNS Topics create the subscriptions, is that they are waiting for confirmation from the SQS queue.

Since we already have a lambda waiting on the other side of the queue, handling all the events, we added a small code to handle the subscription confirmation as well. Here it is:

import json
import urllib.request

def lambda_handler(event, context):
    for record in event["Records"]:
        body = json.loads(record["body"])

        if body.get("Type") == "SubscriptionConfirmation":
            handle_subscription_confirmation(body)

def handle_subscription_confirmation(message):
    url = message["SubscribeURL"]

    with urllib.request.urlopen(url) as response:
        print(response.read())

I find it strange that the Cloudformation template that we use to create the subscription does not handle the confirmation as well. Or maybe not cross-account?


Friday, April 7, 2023

AWS: Create a Layer with Git

 If you want to perform Git operations inside an AWS Lambda written in Python, you can decide to use the GitPython library. However, it needs git to be installed on your system.

In order to package git, I discovered a really nice tool: lambci/yumda. It's a docker container that overrides the yum installer to deploy the library you install, including all its dependencies, into a separate folder called /lambda/opt. So all you need to do is to yum your git, and zip the content of the folder.

Here is an example of a script that packages both GitPython and git into a zip file compatible with AWS Layers:

docker run --rm -v "$PWD":/tmp/layer lambci/yumda:2 bash -c "
  yum install -y git && \
  cd /lambda/opt && \
  zip -yr /tmp/layer/gitlayer.zip
"

pip install GitPython -t python
zip -r gitlayer.zip python/*

Then you can send this zip to an S3 bucket and create your layer easily. The nice thing about running this script inside a CodeBuild is that all tools, such as docker and pip, are already installed. However, for docker, do not forget to enable the Privileged Mode.

Thursday, February 23, 2023

A Case for Zip

 I found this Python code in our project:

data = {...}
devices = get_devices()
if len(devices) > 0:
    data["first_device"] = devices[0]
if len(devices) > 1:
    data["second_device"] = devices[1]
if len(devices) > 2:
    data["third_device"] = devices[2]
if len(devices) > 3:
    data["fourth_device"] = devices[3]
if len(devices) > 4:
    data["fifth_device"] = devices[4]

It retrieves a bunch of devices, and adds them to an existing dict. I removed this code and replaced it with this one:

data = {...}
devices = get_devices()
labels = ["first_device", "second_device", "third_device", "fourth_device", "fifth_device"]
data.update(dict(zip(labels, devices)))

It takes advantage of two characteristics of the zipped list:

  1. It iterates up to the length of the shortest list.
  2. Two zipped lists can easily be turned into a dict.

Monday, January 2, 2023

Warnings on SQS endpoint URL while mocking with moto

 We have many unit tests using moto library to mock calls to boto3 to access AWS services.They all work fine, except for a warning on the use of this fixture:

@fixture(scope='function')
def sqs():
    with mock_sqs():
        yield boto3.client('sqs', region_name=AWS_REGION)

This is one of the correct ways of using moto mocks. But only this mock on SQS shows the following warning:

FutureWarning: The sqs client is currently using a deprecated endpoint: eu-central-1.queue.amazonaws.com. In the next minor version this will be moved to sqs.eu-central-1.amazonaws.com. See https://github.com/boto/botocore/issues/2705 for more details.

 Now, our Sonar is considering these warnings as errors, and leads our build to fail. As explained by the issue, the warning comes from the use of a deprecated attribute called sslCommonName. I didn't dive into the moto library, but there might be a way to avoid the use of this attribute. The easisest way to stop using it, is to add this environment variable to our test code:

import os
os.environ['BOTO_DISABLE_COMMONNAME'] = 'true'

This skips completely the use of the deprecated attribute.

Friday, December 30, 2022

Dict's get has a Default Value

 I found this pattern in several places in our Python code base:

state = "on"
if "State" in data and data["State"] == "off":
    state = "off"

The state here can have two values: on or off. So all this code can be pretty much replaced with this one line:

state = data.get("State", "on")

If the state is missing, it will get the value "on". Of course, you might say that it is not completely equivalent, since anything but "off" would be transformed into "on" in the first code. But then, this code is usually followed by this condition:

if state != "off":

I would have rather replaced the state variable with a boolean, and have a code like this one instead:

state_on = data.get("State") != "off"
if state_on:

If the state is missing, the get would return None, which is also different from "off".

Thursday, October 6, 2022

WTF: Enumeration Galore

 I found some Python code from a guy that used lots of enumeration. He probably copied the code from somewhere and did not understand it well. At the end, we have some pretty confusing code. For example, he wanted to add to a list of trusting accounts some accounts from another list, avoiding duplication. Here is his code:

for i, index in enumerate(account_list):
    if len([a for a in trusting_accounts if a == account_list[i]]) == 0:
        trusting_accounts += [account_list[i]]

The enumerate function returns both the index and the item in the list. However, the fact that he named them i and index shows me that he had no idea what the second part was about. He actually never used it, always referring to the index confusingly called 'i'.

The if condition is even more confusing. It triggers only if the list of accounts having the same name as the current account in the loop is empty. Simply said, it triggers if the account is not already in the list. The corrected version of the code is here:

for account in account_list:
    if account not in trusting_accounts:
        trusting_accounts.append(account)

Of course, this pattern was used in a lots of place in his code. The function for removing a list of accounts from the trusting accounts for instance looks like this:

for i, index in enumerate(account_list):
    for ii, iindex in enumerate(trusting_accounts):
        if account_list[i] == trusting_accounts[ii]:
            trusting_accounts.pop(ii)

There is an inner enumeration to find the index of the account to remove. He probably didn't know that there is a remove function in the list doing all the work. Here is the corrected code:

for account in account_list:
    if account in trusting_accounts:
        trusting_accounts.remove(account)

Much clearer...

Thursday, September 22, 2022

AWS: Unblock CloudFormation stacks from UPDATE_ROLLBACK_FAILED state

 In our project, we have lots of AWS accounts. And we are in charge of deploying base resources in each of them. To do this, we use CloudFormation to deploy several stacks. Actually several stacks nested into one master stack.

One problem we have is that our customers do not always update their stacks to the latest version. Also, when a stack fails to update, they sometimes let it rollback, and do not care to ask for a fix. Of course, this summer, there was even less updates, people being on vacation. And also no release, since we felt there would be nobody to deploy it. So we end up this September with a larger release than usual.

The result of all this is that we found ourselves staring at lots of accounts with a stack entering UPDATE_ROLLBACK_FAILED state. The reason? Deprecation. As it happened, AWS decided to deprecate Python 3.6, and also a couple of Policies (AWSConfigRole, AWSCloudTrailReadOnlyAccess). Of course we updated our stacks with the correct values for Python and the replacement policies some time ago. But as the stacks were not always up to date, and there were some issues while updating to the new release, many stacks started to rollback. And since some of the rollbacked values were deprecated, the rollbacks failed.

When you are in that case, you have two choices. The first one is to delete everything and redeploy. We tried it on one account, and it was really painful. Too many dependencies and manual actions. The second one is the one advised to us by the AWS support itself: continuing rolling back. When you continue a rollback, you have the possibilities to skip some resources. In our case, we needed to skip all lambdas using Python 3.6, and all roles using the deprecated policies.

We tried it manually in the AWS Console, and there is one caveat: you can select resources from nested stacks, but not resources from stacks nested into nested stacks. Since we have many accounts to update, and many resources to rollback, we decided to script the whole process.

We thought it will be simple: using Python and boto3, we list all the resources in our stack, recursively entering nested stacks, and filtering all lambdas and roles. We ran into several problems:

  • You cannot skip resources that are not in a failed stack
  • You cannot skip resources that are not in a failed state
  • You cannot skip resources that are in a failed state because CloudFormation cancelled the update
  • Once you run rollback with skipped resources, CloudFormation discovers new failing resources, so you have to iterate until all is fine, or the list of resources to skip does not change between two iterations.
  • Name of resources in nested stack are <nested_stack_name>.<resource_logical_id>. Even for resources in several level of nested stack, you still use the same pattern, giving only the name of the direct parent stack.
  • Waiting for a rollback to complete will throw an exception if the rollback fails.

This is the script that helps turning a stack from UPDATE_ROLLBACK_FAILED to UPDATE_ROLLBACK_COMPLETE:

import boto3

BLOCKABLE_RESOURCES = [
    "AWS::Lambda::Function",
    "AWS::IAM::Role",
]

STACK_NAME = "MyStack"


def get_stack_status(cf_client, stack_name):
    response = cf_client.describe_stacks(StackName=stack_name)
    return response["Stacks"][0]["StackStatus"]


def find_blocking_resources(cf_client, stack_name, parent, resources):
    response = cf_client.describe_stack_resources(StackName=stack_name)

    if (
        parent
        and get_stack_status(cf_client, stack_name) != "UPDATE_ROLLBAK_FAILED"
    ):
        return

    for resource in response["StackResources"]:
        if (
            resource["ResourceType"] == "AWS::CloudFormation::Stack"
            and resource["ResourceStatus"] == "UPDATE_FAILED"
        ):
            nested_name = resource["PhysicalResourceId"].split("/")[1]
            find_blocking_resources(
                cf_client, nested_name, nested_name + ".", resources
            )
        elif (
            resource["ResourceType"] in BLOCKABLE_RESOURCES
            and resource["ResourceStatus"] == "UPDATE_FAILED"
            and resource.get("ResourceStatusReason")
            != "Resource update cancelled"
        ):
            resource.append(parent + resource["LogicalResourceId"])


cf_client = boto3.client("cloudformation")

status = get_stack_status(cf_client, STACK_NAME)

if status != "UPDATE_ROLLBACK_FAILED":
    print("Nothing to unblock. Exiting")
    exit(0)

resources = []
previous_resources = ["DUMMY"]
waiter = cf_client.get_waiter("stack_rollback_complete")

print("Starting unblocking process")
while status != "UPDATE_ROLLBACK_FAILED" and previous_resources != resources:
    previous_resources = resources
    resources = []
    find_blocking_resources(cf_client, STACK_NAME, "", resources)
    print("Skipping ", resources)

    cf_client.continue_update_rollback(
        StackName=STACK_NAME, ResourcesToSkip=resources
    )
    try:
        waiter.wait(StackName=STACK_NAME)
    except Exception as err:
        print(err)

    status = get_stack_status(cf_client, STACK_NAME)

print("Final stack status:", status)

Of course, once this is done, you still have to fix the stacks and run an update.

Thursday, August 25, 2022

Modifying AWS resource for testing

 If you are writing unit tests using boto3 and moto, you might want to modify a value in a resource. I had this case, for instance, where I wanted to set the state of an AMI to pending for a test. Using moto, AMIs are always created in the available state. However, if you try to do it this way, it will fail:

ec2 = boto3.resource("ec2")
image = ec2.Image(AMI_ID)
image.state = "pending"

You will end up with this message:

AttributeError: can't set attribute 'state'

It is possible to modify the resource object. However, it will just modify this object instance, the model itself will not be updated. So if you use the resource as a parameter to a function, it will work. But if the function retrieves its data with boto3, it will get the original value.

Here is my solution:

image.meta.data.update({"State": "pending"})

That helped in my case. 

Monday, June 13, 2022

WTF: Case for a Join

 I found this code in our repository:

roles = ""
for i, id in enumerate(account_ids):
    if i > 0:
        roles = roles + ","
        if i == len(account_ids) - 1:
            roles = roles + "\"arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin"
        else:
            roles = roles + "\"arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin\""
    if i == 0:
        if i == len(account_ids) - 1:
            roles = roles + "arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin"
        else:
            roles = roles + "arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin\""

It takes some time to understand what is going on here, but basically, we are building a coma separated list of AWS roles from a list of account IDs.

When you see a condition based on the index of a for loop, you start to feel a very strong code smell.

First, the test for the index being bigger than zero, or equal to 0, is mainly made for handling the coma. But not only. There are then tests repeated, with almost similar codes, to check if we are on the last iteration, all this to discover if we have to add a " character at the beginning or the end of our string.

At the end, it produces a string in the form: role1","role2","role3

The reason that there is no quotation marks at the beginning or the end of the string, is that ultimately, it will be put in a template that is declared with the marks already there, in this form: "__ROLES__".

All this code is quite bad, and the coma and quotation marks handling can all be left to a call to the join function:

roles = '","'.join([f"arn:aws:iam::*:role/{entity_name}-{id}-admin"
    for id in account_ids])

From 13 lines to 1, I kind of like it.

Monday, May 16, 2022

WTF: Python Dict two liner

 When working with DynamoDB in AWS, you know that the values you put in there are not your usual JSON. Instead of you key/value pair, your rather have key/type/value. Something like this:

{
    'key': {
        'type': 'value'
    }
}

Of course, you can let your API build this mess for you. Or you can do it yourself, as I found in our code base. Except it had this little gem in it. For creating a map, you have to apply this key/type/value pattern to all values. And then you have to insert the map using the 'M' type. The code ended with these two lines:

data = dict({})
data['M'] = my_map

First, dict({}) is like creating en empty dict from an empty dict. You either use dict() or {}, but not both. But since you can create your dict inline, I'd rather use this one-liner:

data = {'M': my_map}

Does it look simpler only to me?

Thursday, April 21, 2022

WTF: Oops, already in my list

 I found an interesting loop pattern, used at several places in our code. It goes like this: 

  • I create an empty list
  • I start a loop to fill it
  • Inside my loop, I create an object and add it to my list
  • Still in the loop, I need to modify the object I just added, so I retrieve it from the list
An example here:
templates = []
index = 0

json_templates = json.loads(templates_as_str)
for template_file in json_templates:
    templates.add(get_template(template_file))

    template = templates[index]
    # Do some stuff on my template

    index += 1
So yes, I could have created my template reference before storing it in the list. But it's too late! I need to use an index now...

Wednesday, April 6, 2022

Python: Over formatting

In the Python project I work on, some colleague likes to use the string's format method. Sometimes a bit too much for my taste. For instance, I can often see this kind of code:

my_string = "{}-{}".format(part1, part2)

Usually, I prefer to use an f-string:

my_string = f"{part1}-{part2}"

But it could be a matter of taste. However, in some cases, I would prefer to use another approach.

For instance, in the case of the print method, there is already an existing pattern. I see often this kind of code:

print("The value is {}".format(value))

I replace it usually with this code:

print("The value is", value)

A bit more annoying is the use of format inside logging. I often see this code:

logger.info("The value is {}".format(value))

In case of logging, the pattern is here for a reason. If logging level is set to WARNING for instance, you want to avoid the string formatting, which takes some processing time. The preferred pattern is the following:

logger.info("The value is %s", value)

Finally, there are the cases where the use of format is completely insane. Here is an example:

my_string = "{}".format(value)

Maybe my knowledge of Python is too limited, but value being already a string, is there any difference with this code:

my_string = value