pynamodb / PynamoDB

A pythonic interface to Amazon's DynamoDB
http://pynamodb.readthedocs.io
MIT License
2.41k stars 428 forks source link

v5.2.0 breaking change? AttributeError: type object 'Meta' has no attribute 'read_capacity_units' #1017

Closed lehmat closed 2 years ago

lehmat commented 2 years ago

The release of 5.2.0 broke GlobalSecondaryIndex syntax that worked on 5.1.0. Requesting to add a note about it in release notes for 5.2.0

Broken syntax in 5.2.0, but working in 5.1.0

class UserRangeKey(GlobalSecondaryIndex):
    class Meta:
        index_name = "user-range-key-index"
        projection = AllProjection()

    user_range_key = UnicodeAttribute(hash_key=True)

Valid syntax in 5.2.0

class UserRangeKey(GlobalSecondaryIndex):
    class Meta:
        index_name = "user-range-key-index"
        projection = AllProjection()
        billing_mode = "PAY_PER_REQUEST"

    user_range_key = UnicodeAttribute(hash_key=True)

Lines it breaks in is in PR #996

ikonst commented 2 years ago

Must've been unintentional. Gotta look into this.

ikonst commented 2 years ago

@lehmat What is the stack trace where this error happens for you?

lehmat commented 2 years ago
/python/lib/python3.8/site-packages/pynamodb/models.py:796: in create_table
    schema = cls._get_schema()
/python/lib/python3.8/site-packages/pynamodb/models.py:880: in _get_schema
    index_schema = index._get_schema()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

cls = <class '<filtered>....UserRangeKey'>

    @classmethod
    def _get_schema(cls) -> Dict:
        schema = super()._get_schema()
        if getattr(cls.Meta, 'billing_mode', None) != PAY_PER_REQUEST_BILLING_MODE:
            schema['provisioned_throughput'] = {
>               READ_CAPACITY_UNITS: cls.Meta.read_capacity_units,
                WRITE_CAPACITY_UNITS: cls.Meta.write_capacity_units
            }
E           AttributeError: type object 'Meta' has no attribute 'read_capacity_units'
ikonst commented 2 years ago

The problem reproduces for me with both 5.1.0 and 5.2.0, although it crashes in a slightly different place, but with the same error.

The full code:

from pynamodb.attributes import UnicodeAttribute
from pynamodb.indexes import GlobalSecondaryIndex, AllProjection
from pynamodb.models import Model

class UserRangeKey(GlobalSecondaryIndex):
    class Meta:
        index_name = "user-range-key-index"
        projection = AllProjection()

    user_range_key = UnicodeAttribute(hash_key=True)

class MyModel(Model):
    class Meta:
        host = 'http://localhost:8000'
        table_name = 'foo'
        aws_access_key_id = ' foo'
        aws_session_token = 'foo'
        aws_secret_access_key = 'foo'

    user_range_key = UserRangeKey()

MyModel.create_table()
nirajvbhatt commented 2 years ago

Facing same issue

ikonst commented 2 years ago

@nirajvbhatt Do you have simple code which reproduces the problem on 5.2 but not 5.1?

nirajvbhatt commented 2 years ago

Hey @ikonst , sorry for late reply. I couldn't reproduce the issue on 5.2.

jovana commented 2 years ago

@nirajvbhatt Do you have simple code which reproduces the problem on 5.2 but not 5.1?

I have the same issue. Below is simple code:

from pynamodb.models import Model
from pynamodb.indexes import GlobalSecondaryIndex, AllProjection
from pynamodb.attributes import (
    UnicodeAttribute,
    BooleanAttribute,
    NumberAttribute
)
import os

class TradingHistoryDatabaseModel(Model):

    class Meta:
        table_name = "trading_history"
        region = "eu-west-1"
        billing_mode = "PAY_PER_REQUEST"

    class StrategyBotIdIndex(GlobalSecondaryIndex):
        class Meta:
            projection = AllProjection()
        strategy_bot_id = UnicodeAttribute(hash_key=True)

    history_id = UnicodeAttribute(hash_key=True)
    history_type = UnicodeAttribute(range_key=True) 
    customer_id = UnicodeAttribute()
    strategy_id = UnicodeAttribute()
    strategy_bot_id = UnicodeAttribute()
    strategy_bot_id_index = StrategyBotIdIndex()

if not TradingHistoryDatabaseModel.exists():
    TradingHistoryDatabaseModel.create_table(wait=True)

Stack trace:

ERROR:    Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 540, in lifespan
    async for item in self.lifespan_context(app):
  File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 481, in default_lifespan
    await self.startup()
  File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 516, in startup
    await handler()
  File "/app/./main.py", line 45, in startup_event
    StartingApplication().CheckingDatabaseTables()
  File "/app/./application/utils/events/starting.py", line 87, in CheckingDatabaseTables
    TradingHistoryDatabaseModel.create_table(wait=True)
  File "/usr/local/lib/python3.9/site-packages/pynamodb/models.py", line 796, in create_table
    schema = cls._get_schema()
  File "/usr/local/lib/python3.9/site-packages/pynamodb/models.py", line 880, in _get_schema
    index_schema = index._get_schema()
  File "/usr/local/lib/python3.9/site-packages/pynamodb/indexes.py", line 181, in _get_schema
    READ_CAPACITY_UNITS: cls.Meta.read_capacity_units,
AttributeError: type object 'Meta' has no attribute 'read_capacity_units'
jovana commented 2 years ago

Output from pip freeze

anyio==3.5.0
APScheduler==3.8.1
asgiref==3.4.1
asttokens==2.0.5
async-generator==1.10
attrs==21.4.0
bech32==1.2.0
boto3==1.20.49
botocore==1.23.49
certifi==2021.10.8
charset-normalizer==2.0.11
click==8.0.1
colorama==0.4.4
dnspython==2.2.0
ecdsa==0.17.0
email-validator==1.1.3
executing==0.8.2
fastapi==0.68.1
gunicorn==20.1.0
h11==0.12.0
httpcore==0.14.7
httptools==0.2.0
httpx==0.22.0
icecream==2.1.1
idna==3.3
jmespath==0.10.0
lnurl==0.3.6
outcome==1.1.0
pyasn1==0.4.8
pydantic==1.8.2
Pygments==2.11.2
pynamodb==5.2.0
python-dateutil==2.8.2
python-dotenv==0.19.0
python-jose==3.3.0
python-multipart==0.0.5
pytz==2021.3
pytz-deprecation-shim==0.1.0.post0
PyYAML==5.4.1
requests==2.27.1
rfc3986==1.5.0
rsa==4.8
s3transfer==0.5.1
simplejson==3.17.6
six==1.16.0
sniffio==1.2.0
sortedcontainers==2.4.0
starlette==0.14.2
trio==0.19.0
typing-extensions==3.10.0.2
tzdata==2021.5
tzlocal==4.1
urllib3==1.26.8
uvicorn==0.15.0
uvloop==0.16.0
watchgod==0.7
websockets==10.0
ikonst commented 2 years ago

Ahh, you guys are right. I think the culprit is this:

We used to check for the model's billing mode: https://github.com/pynamodb/PynamoDB/blob/4bbfad68d5053f4adcbd503560efa901bae81315/pynamodb/models.py#L908

but now we're expecting the index to have a billing mode: https://github.com/pynamodb/PynamoDB/blob/11113283c92a13566515dcd2f514c3aefda8609a/pynamodb/indexes.py#L175

lehmat commented 2 years ago

You are correct @ikonst. The ticket was about adding a notice in the releases to avoid people looking at the releases and come to a conclusion that no change is required.

Overall I think it is good to be able to being able to define billing mode for the global secondary index

jovana commented 2 years ago

Ahh, you guys are right. I think the culprit is this:

We used to check for the model's billing mode:

https://github.com/pynamodb/PynamoDB/blob/4bbfad68d5053f4adcbd503560efa901bae81315/pynamodb/models.py#L908

but now we're expecting the index to have a billing mode:

https://github.com/pynamodb/PynamoDB/blob/11113283c92a13566515dcd2f514c3aefda8609a/pynamodb/indexes.py#L175

Cool, thx for the solution!!

ikonst commented 2 years ago

@lehmat per UpdateTable docs, you cannot define a billing mode just for an index - it's always per the entire table and all its indices. So this 5.2.0 change looks like a bug that we need to fix.

jpinner-lyft commented 2 years ago

@ikonst Agree this is a bug to be fixed -- really we want to check the billing mode of the table to see if we should add provisioned throughput to the index settings. We also might want to change the checks to explicitly check for the "PROVISIONED" billing mode.

jpinner-lyft commented 2 years ago

For reference the original logic correctly looked at the model Meta:

-                if isinstance(index, GlobalSecondaryIndex):
-                    if getattr(cls.Meta, 'billing_mode', None) != PAY_PER_REQUEST_BILLING_MODE:
-                        idx['provisioned_throughput'] = {
-                            READ_CAPACITY_UNITS: index.Meta.read_capacity_units,
-                            WRITE_CAPACITY_UNITS: index.Meta.write_capacity_units
-                        }
lehmat commented 2 years ago

Am I missing something? https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-dynamodb-gsi.html#cfn-dynamodb-gsi-provisionedthroughput, you can specify provisioned throughput for global secondary indexes

ikonst commented 2 years ago

@lehmat You can specify a separate throughput for each GSI, but not separate billing mode. Billing mode is a table-level setting.

garrettheel commented 2 years ago

Note that I've backported the fix from #1018 in a 5.2.1 release: https://pypi.org/project/pynamodb/5.2.1/