Skip to content
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

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

Closed
lehmat opened this issue Jan 12, 2022 · 18 comments · Fixed by #1157

Comments

@lehmat
Copy link

lehmat commented Jan 12, 2022

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
Copy link
Contributor

ikonst commented Jan 12, 2022

Must've been unintentional. Gotta look into this.

@ikonst
Copy link
Contributor

ikonst commented Jan 12, 2022

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

@lehmat
Copy link
Author

lehmat commented Jan 13, 2022

/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
Copy link
Contributor

ikonst commented Jan 13, 2022

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
Copy link

Facing same issue

@ikonst
Copy link
Contributor

ikonst commented Jan 28, 2022

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

@nirajvbhatt
Copy link

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

@jovana
Copy link
Contributor

jovana commented Feb 6, 2022

@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
Copy link
Contributor

jovana commented Feb 6, 2022

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
Copy link
Contributor

ikonst commented Feb 7, 2022

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

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

if getattr(cls.Meta, 'billing_mode', None) != PAY_PER_REQUEST_BILLING_MODE:

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

if getattr(cls.Meta, 'billing_mode', None) != PAY_PER_REQUEST_BILLING_MODE:

@lehmat
Copy link
Author

lehmat commented Feb 7, 2022

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
Copy link
Contributor

jovana commented Feb 7, 2022

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

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

if getattr(cls.Meta, 'billing_mode', None) != PAY_PER_REQUEST_BILLING_MODE:

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

if getattr(cls.Meta, 'billing_mode', None) != PAY_PER_REQUEST_BILLING_MODE:

Cool, thx for the solution!!

@ikonst
Copy link
Contributor

ikonst commented Feb 7, 2022

@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
Copy link
Contributor

jpinner-lyft commented Feb 7, 2022

@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
Copy link
Contributor

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
Copy link
Author

lehmat commented Feb 7, 2022

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
Copy link
Contributor

ikonst commented Feb 7, 2022

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

@garrettheel
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants