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

QuerySelectField returning ValueError: too many values to unpack (expected 2) #9

Closed
tjcim opened this issue Jan 2, 2018 · 18 comments
Closed
Milestone

Comments

@tjcim
Copy link

tjcim commented Jan 2, 2018

Environment

Python 3.6.3

click==6.7
Flask==0.12.2
Flask-SQLAlchemy==2.3.2
Flask-WTF==0.14.2
itsdangerous==0.24
Jinja2==2.10
MarkupSafe==1.0
SQLAlchemy==1.2.0
Werkzeug==0.14.1
WTForms==2.1
WTForms-SQLAlchemy==0.1

How to recreate:

app.py

import os
from flask import Flask, render_template
from flask_sqlalchemy import SQLAlchemy
from flask_wtf import FlaskForm
from wtforms_sqlalchemy.fields import QuerySelectField


app = Flask(__name__)
basedir = os.path.abspath(os.path.dirname(__file__))

app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///' + os.path.join(basedir, 'test.db')
app.config['SECRET_KEY'] = 'secret'


db = SQLAlchemy(app)


class Choice(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.String(50))
    extra = db.Column(db.String(50))


def choice_query():
    return Choice.query


class ChoiceForm(FlaskForm):
    opts = QuerySelectField(query_factory=choice_query, allow_blank=False)


@app.route('/')
def index():
    form = ChoiceForm()
    return render_template('index.html', form=form)


if __name__ == '__main__':
    db.create_all()
    one = Choice(name='one')
    two = Choice(name='two')
    db.session.add(one)
    db.session.add(two)
    db.session.commit()
    app.run(debug=True)

templates/index.html

  <head>
    <title>Form</title>
  </head>
  <body>
    <form action="/" method="POST">
      {{ form.csrf_token }}
      {{ form.opts }}
      <ul>
        {% for error in form.opts.errors %}
        <li style="color:red;">{{ error }}</li>
        {% endfor %}
      </ul>
      <input type="submit">
    </form>
  </body>
</html>
@tjcim
Copy link
Author

tjcim commented Jan 2, 2018

I don't know how to fix it properly, because I don't understand the change (zzzeek/sqlalchemy@50d9f16#diff-f1dd179c3cad67b54ed3deb8e6f6fa6b) but if anyone else comes across this error before it is corrected I did this:

Edited wtforms_sqlalchemy/fields.py and changed line 189 from cls, key = identity_key(instance=obj) to cls, key, _ = identity_key(instance=obj)

@brookskindle
Copy link
Contributor

brookskindle commented Jan 4, 2018

You're a day ahead of me, @tjcim, good catch! I also noticed the problem but didn't figure it out until today.
According to the SQLAlchemy changelog for 1.2, my understanding is that it has something to do with differentiating objects with the same primary key but across a sharded database.

Currently my solution is to keep SQLAlchemy below 1.2 but that's not a preferable solution long term.

Someone more familiar with the wtforms codebase (@pawl, @prencher, @davidism?) might have a better idea of how to fix it, but it seem like one possible solution would be to modify get_pk_from_identity

def get_pk_from_identity(obj):
    cls, key = identity_key(instance=obj)
    return ':'.join(text_type(x) for x in key)

to somehow include the identity_token in its return value? Maybe like so

def get_pk_from_identity(obj):
    cls, key, token = identity_key(instance=obj)
    return ':'.join("{}-{}".format(text_type(x), token) for x in key)

But again I don't know enough of wtforms internals to know where this kind of a change would cause problems

@fuhrysteve
Copy link

#10 resolves this issue and is backwards compatible with older versions of SQLAlchemy

@tjcim
Copy link
Author

tjcim commented Jan 17, 2018

@fuhrysteve - What is the advantage of doing cls, key = identity_key(instance=obj)[:2] as opposed to what I did above cls, key, _ = identity_key(instance=obj)

I am trying to get better, and was just curious what the advantages are.

@fuhrysteve
Copy link

@tjcim Well for starters, cls, key, _ = identity_key(instance=obj) would not work on sqlalchemy versions prior to 1.2, since it would fail when identity_key only returns 2 values.

>>> a, b, c = (1, 2)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-1-24077ba23852> in <module>()
----> 1 a, b, c = (1, 2)

ValueError: not enough values to unpack (expected 3, got 2)

See this for example:

>>> a, b = (1, 2, 3)[:2]  # This works!

>>> a, b = (1, 2, 3)  # This won't work :(
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-faee755feda6> in <module>()
----> 1 a, b = (1, 2, 3)

ValueError: too many values to unpack (expected 2)

@tjcim
Copy link
Author

tjcim commented Jan 17, 2018

@fuhrysteve - yup, that makes sense. Thank you.

@Cheaterman
Copy link

Also, if this API was unstable in the past, there's no reason it can't change again in the future, so the best we can do is assume we'll only get more arguments appended to the list as opposed to prepended. (That's the reasoning behind the PR we did with tshirtman)

@brookskindle
Copy link
Contributor

I'd like this to be fixed soon, too. The README says that it's looking for a new maintainer, so I've emailed offering to maintain the project.

Regardless of if that ends up being the case, it looks like progress is being made on getting this issue fixed, per @davidism in wtforms/wtforms#373 (comment)

@davidism
Copy link
Member

davidism commented Mar 5, 2018

What README says that? Please don't contact the old WTForms maintainers, they have moved on and will just be confused if they get more people pinging them. Anyway, this is an issue with WTForms, wtforms/wtforms#373, so closing here.

@davidism davidism closed this as completed Mar 5, 2018
@brookskindle
Copy link
Contributor

This project's readme states it's looking for a maintainer.

This package is currently looking for a permanent maintainer, and if you wish to maintain this package please contact wtforms@simplecodes.com.

If that's not the case, then the README is incorrect. I have a vested interest in seeing this issue fixed, so I had taken steps I thought would ensure a path to a fix.


I'm all for having one spot to track the issue, so if you think wtforms/wtforms#373 is the best place to track it, then I don't disagree.

As a heads up in case you missed it, #10 has a fix for this issue, though I know there were a couple of other solutions presented in wtforms/wtforms#373.

@davidism
Copy link
Member

davidism commented Mar 5, 2018

Oh huh, didn't even notice this was the WTForms-SQLAlchemy repo, thought it was the Flask-WTF one. Is there a reason you're using this over WTForms-Alchemy, which has been maintained more actively? I don't really think I want to revive this specific repo.

@brookskindle
Copy link
Contributor

Is there a reason you're using this over WTForms-Alchemy, which has been maintained more actively?

Historical reasons, probably.

I don't use the this library directly, but projects that I maintain at work use wtforms and, by extension (hah!), wtforms-sqlalchemy since it's bundled in with wtforms.

I discovered wtforms-alchemy only recently and haven't really had a chance to look at it. If it's a drop-in replacement for wtforms-sqlalchemy then I'm fine using either one. Though, if you want to deprecate wtforms-sqlalchemy, shouldn't wtforms include/reference wtforms-alchemy instead?

@davidism
Copy link
Member

davidism commented Mar 5, 2018

It's not bundled in WTForms. It's that part of WTForms extracted, since WTForms was going to remove it. Prefer and update the WTForms version for now.

@brookskindle
Copy link
Contributor

I'm relying on the most current version of wtforms listed on pypi, 2.1, but I hear that's another issue you're working on. Looks like kvesteri/wtforms-alchemy#128 has already identified and fixed the sqlalchemy issue, I'll have to look more seriously into switching libraries, then.

Thanks for your explanations and suggestions.

magical added a commit to magical/spline-pokedex that referenced this issue Apr 17, 2018
@davidism davidism reopened this May 17, 2018
@davidism
Copy link
Member

Going to fix this here and in WTForms (for now, it's going away in WTForms eventually). I was definitely confused about which project was which here. This or WTForms-Alchemy should have the SQLAlchemy part of WTForms.

@jrborbars
Copy link

@davidism this message was showed in my app after tests.
/home/ubuntu/.virtualenvs/workspace/lib/python3.4/site-packages/wtforms/ext/sqlalchemy/init.py:9: DeprecationWarning: wtforms.ext.sqlalchemy is deprecated, and will be removed in WTForms 3.0. The package has been extracted to a separate package wtforms_sqlalchemy: https://github.com/wtforms/wtforms-sqlalchemy .
Or alternately, check out the WTForms-Alchemy package which provides declarative mapping and more: https://github.com/kvesteri/wtforms-alchemy
DeprecationWarning

@davidism
Copy link
Member

closed by #10 and #11

@davidism davidism added this to the 0.2 milestone Jun 29, 2018
@wtforms wtforms deleted a comment from batyrata Aug 3, 2018
@ilpssun
Copy link

ilpssun commented Jan 21, 2019

This seems to be fixed in master but the fix does not seem to be part of any release, yet.

@davidism When is the next release planned?

mx-moth pushed a commit to mx-moth/wtforms-sqlalchemy that referenced this issue Feb 7, 2020
Do not assume not-nullable means required for Boolean columns
mx-moth pushed a commit to mx-moth/wtforms-sqlalchemy that referenced this issue Feb 7, 2020
epikt added a commit to jpgil/cl-concerts-db that referenced this issue Feb 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants