Skip to content

How can I roll back after a database error? #293

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

Closed
mRcSchwering opened this issue Oct 14, 2020 · 9 comments
Closed

How can I roll back after a database error? #293

mRcSchwering opened this issue Oct 14, 2020 · 9 comments

Comments

@mRcSchwering
Copy link

Hi, I am using this pattern: https://docs.graphene-python.org/projects/sqlalchemy/en/latest/tutorial/#defining-our-models

I noticed that when the database runs into an error, any further queries that want to use the same session are blocked.
E.g.:

  1. Have a postgres database with a table that has a date field and define queries.
  2. Now try to query the table and filter for the date.
  3. For the date, you accidentally enter an int (a year). You will run into an error like this:
sqlalchemy.exc.DataError: (psycopg2.errors.InvalidDatetimeFormat) invalid input syntax for type date: "2001"
  1. Now try a new query (correctly formed), you will get this error:
graphql.error.located_error.GraphQLLocatedError: (psycopg2.errors.InFailedSqlTransaction) current transaction is aborted, commands ignored until end of transaction block

The session is basically blocked and since the session management is hidden I dont know how to roll back the failed transaction from 3.

@chrisberks
Copy link
Contributor

How are you passing the invalid type to the query? GraphQL should guard against this.

@mRcSchwering
Copy link
Author

That's true. So, that might be another issue. My code looked like this:

# models.py
engine = create_engine(SQLALCHEMY_DATABASE_URI)
SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)

Base = declarative_base()
Base.query = scoped_session(SessionLocal).query_property()


class Reference(Base):
    dbid = Column(Integer, primary_key=True, index=True)
    date = Column(Date, nullable=False)

    title = Column(Text)
    label = Column(String)


# schema.py
class Reference(SQLAlchemyObjectType):
    class Meta:
        model = models.Reference
        interfaces = (graphene.relay.Node,)


class CreateReference(graphene.Mutation):
    class Arguments:
        date = graphene.Date()
        title = graphene.String()
        label = graphene.String(required=False)

    ok = graphene.Boolean()
    node = graphene.Field(lambda: Reference)

    @staticmethod
    def mutate(*_, **kwargs):
        with get_db() as db:
            node = crud.create_or_update_reference(db=db, **kwargs)
        return CreateReference(node=node, ok=True)

@chrisberks
Copy link
Contributor

Ah. I understand now, thanks for clarifying.

It looks like graphene-sqlalchemy converts sqlalchemy.types.Date and sqlalchemy.types.Time to graphene.String rather than the GraphQL scalar types graphene.Date and graphene.Time.

If I'm right, that this is what's causing the problem, a quick fix might be to use ORMField to override the date field's type.

How are you implementing your filtering?

# schema.py
from graphene_sqlalchemy.types import ORMField

class Reference(SQLAlchemyObjectType):
    class Meta:
        model = models.Reference
        interfaces = (graphene.relay.Node,)

    date = ORMField(type=graphene.Date)

@erikwrede
Copy link
Member

Should we change that behavior to convert sqlalchemy.types.Date and sqlalchemy.types.Time to graphene.Date and graphene.Time, or is there any reason not to do that?

@chrisberks
Copy link
Contributor

Thanks for picking this up @erikwrede.

I see no reason this shouldn't be the case, graphene-django does what you're suggesting. I assume this would be a breaking change and need to be added to the next major release.

@erikwrede
Copy link
Member

I think we could easily add that to the graphene-sqlalchemy v3 release since it is a major release anyways. It should be a 4 liner in the codebase.

@erikwrede
Copy link
Member

erikwrede commented Apr 28, 2022

I've created a new issue collecting all the type converter problems/suggestions. Feel free to add more!

@erikwrede erikwrede self-assigned this Jun 9, 2022
@erikwrede
Copy link
Member

Made the changes in #353. Closing this for now. If new Issues occur, please open another Issue!

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related topics referencing this issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants