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

0.6.3 + entity.flush(): after_insert() does not work #161

Closed
socketpair opened this Issue Feb 10, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@socketpair

socketpair commented Feb 10, 2016

This code does not call after_insert() hook. Also cannot check if that ValueError will be seen in except ValueError as e

from pony.orm import Database, db_session

db = Database()

class TestTable(db.Entity):
    def after_insert(self):
        raise ValueError("a_i")

db.bind("sqlite", ":memory:")
db.generate_mapping(create_tables=True)
try:
    with db_session:
        entity = TestTable()
        entity.flush()
except ValueError as e:
    print("ValueError raised", e)
@socketpair

This comment has been minimized.

Show comment
Hide comment
@socketpair

socketpair Feb 10, 2016

Мы начинаем нервно посматривать на другие ORM. Ребят, ей богу, ну вот только что же говорили перед выпуском 0.6.3 о проблеме с flush. можно же тесты сделать, наверно? Я переживаю, что код стал слишком сложным и вы в нём сами запутались. других объяснений я не нахожу :(

  1. Если entity.flush() вообще убрать - то вместо обещанного ValueError будет pony.orm.core.CommitException: ValueError: a_i
  2. Если вставить глобальный flush() то будет таки ValueError

но документация говорит, что commit (пусть и не явный) делает внутри flush(). чё-то не сходится....

socketpair commented Feb 10, 2016

Мы начинаем нервно посматривать на другие ORM. Ребят, ей богу, ну вот только что же говорили перед выпуском 0.6.3 о проблеме с flush. можно же тесты сделать, наверно? Я переживаю, что код стал слишком сложным и вы в нём сами запутались. других объяснений я не нахожу :(

  1. Если entity.flush() вообще убрать - то вместо обещанного ValueError будет pony.orm.core.CommitException: ValueError: a_i
  2. Если вставить глобальный flush() то будет таки ValueError

но документация говорит, что commit (пусть и не явный) делает внутри flush(). чё-то не сходится....

@socketpair

This comment has been minimized.

Show comment
Hide comment
@socketpair

socketpair Feb 10, 2016

насчёт *_update() мы ещё не проверяли.

socketpair commented Feb 10, 2016

насчёт *_update() мы ещё не проверяли.

@kozlovsky kozlovsky added the bug label Feb 10, 2016

@kozlovsky kozlovsky added this to the 0.6.4 milestone Feb 10, 2016

@kozlovsky kozlovsky self-assigned this Feb 10, 2016

@kozlovsky kozlovsky closed this in 4e23910 Feb 10, 2016

@kozlovsky

This comment has been minimized.

Show comment
Hide comment
@kozlovsky

kozlovsky Feb 10, 2016

Member

Sorry for the inconvenience. As it turned out, the code of obj.flush() was not tested properly. Now all the hooks should work correctly after obj.flush() as well as after the global flush() function. We will release 0.6.4. today with this fix.

Member

kozlovsky commented Feb 10, 2016

Sorry for the inconvenience. As it turned out, the code of obj.flush() was not tested properly. Now all the hooks should work correctly after obj.flush() as well as after the global flush() function. We will release 0.6.4. today with this fix.

@socketpair

This comment has been minimized.

Show comment
Hide comment
@socketpair

socketpair Feb 10, 2016

Очень здОрово, что вы фиксите баги так быстро! Очень рад таки, а то у нас релз и под самый релиз такое дело.

А с видом исключения поведение пофикшено или так и будет как я описал в баге ?

socketpair commented Feb 10, 2016

Очень здОрово, что вы фиксите баги так быстро! Очень рад таки, а то у нас релз и под самый релиз такое дело.

А с видом исключения поведение пофикшено или так и будет как я описал в баге ?

@kozlovsky

This comment has been minimized.

Show comment
Hide comment
@kozlovsky

kozlovsky Feb 10, 2016

Member

Я думаю что текущие баги с релизом - это просто "болезнь роста" - у нас сейчас резко возросло число core developers, работающих над Пони full time, и сейчас идет взаимная притирка. Ожидаю, что в ближайшее время появится и поддержка jsonb в PostgreSQL, и upserts, и много чего еще.

Что касается исключения, то оно будет успешно перехвачено. Поскольку flush в приведенном примере кода происходит явным образом за пределами commit, то исключение не будет обернуто в CommitException, а так и останется ValueError.

Если сейчас речь о #159, чтобы никогда не обертывать исключения, происходящие во flush, в CommitException, то мы не могли сделать это в текущем hotfix-релизе, потому что такое изменение ломает обратную совместимость. Оно будет частью следующего релиза 0.6.5. Сейчас при необходимости можно явно вызывать flush() перед выполнением commit или выходом из db_session

Member

kozlovsky commented Feb 10, 2016

Я думаю что текущие баги с релизом - это просто "болезнь роста" - у нас сейчас резко возросло число core developers, работающих над Пони full time, и сейчас идет взаимная притирка. Ожидаю, что в ближайшее время появится и поддержка jsonb в PostgreSQL, и upserts, и много чего еще.

Что касается исключения, то оно будет успешно перехвачено. Поскольку flush в приведенном примере кода происходит явным образом за пределами commit, то исключение не будет обернуто в CommitException, а так и останется ValueError.

Если сейчас речь о #159, чтобы никогда не обертывать исключения, происходящие во flush, в CommitException, то мы не могли сделать это в текущем hotfix-релизе, потому что такое изменение ломает обратную совместимость. Оно будет частью следующего релиза 0.6.5. Сейчас при необходимости можно явно вызывать flush() перед выполнением commit или выходом из db_session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment