Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: The default updated_at when a workflow is created #11709

Merged
merged 1 commit into from
Dec 21, 2024

Conversation

jiangbo721
Copy link
Contributor

Summary

fix: The default updated_at when a workflow is created is the time the service was started.
The adjusted default updated_at is the current time.

Tip

Close issue syntax: Fixes #<issue number> or Resolves #<issue number>, see documentation for more details.

Screenshots

Before After
... ...

Checklist

Important

Please review the checklist below before submitting your pull request.

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🐞 bug Something isn't working labels Dec 16, 2024
@jiangbo721
Copy link
Contributor Author

When creating an app, the updated_at time in the top corner is always a fixed time, and this should be the time when the service was started
image

@crazywoola crazywoola requested a review from laipz8200 December 19, 2024 04:32
api/models/workflow.py Outdated Show resolved Hide resolved
@jiangbo721 jiangbo721 force-pushed the fix/workflow_updated_at branch 2 times, most recently from de70b50 to 6f3505f Compare December 19, 2024 09:17
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Dec 19, 2024
@jiangbo721 jiangbo721 force-pushed the fix/workflow_updated_at branch 4 times, most recently from 361b41a to 49934eb Compare December 19, 2024 11:17
Copy link
Contributor Author

@jiangbo721 jiangbo721 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biggest difference is the use of server_default instead of default to avoid updated_at becoming the server startup time.I think func.current_timestamp() is better than db.text(‘CURRENT_TIMESTAMP(0)’) for cross-database auto-adaptation.
So I replaced both created_at and updated_at

@jiangbo721 jiangbo721 requested a review from laipz8200 December 19, 2024 12:22
@laipz8200
Copy link
Member

Add server_default requires a database migration, please wait for our refactor in the models module in #11838.

@laipz8200
Copy link
Member

Hello @jiangbo721. #11838 has been merged, can you sync your code with the main branch and generate a new migration for this PR?

The migration file can be generated by running flask db migrate -m "your comment".

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Dec 20, 2024
@jiangbo721 jiangbo721 force-pushed the fix/workflow_updated_at branch from 282ea0b to f21e8bd Compare December 20, 2024 08:19
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Dec 20, 2024
Copy link
Contributor Author

@jiangbo721 jiangbo721 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes in schema detected.

@jiangbo721 jiangbo721 force-pushed the fix/workflow_updated_at branch from f21e8bd to 8973643 Compare December 20, 2024 12:57
…e service was started.

The adjusted default updated_at is the current time.
@jiangbo721 jiangbo721 force-pushed the fix/workflow_updated_at branch from 8973643 to a73f28e Compare December 20, 2024 13:30
Copy link
Member

@laipz8200 laipz8200 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 21, 2024
@laipz8200 laipz8200 merged commit 9578246 into langgenius:main Dec 21, 2024
5 checks passed
@yihong0618
Copy link
Contributor

after this PR
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hyi/pr/dify/api/services/workflow_service.py", line 127, in sync_draft_workflow
db.session.commit()
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/scoping.py", line 597, in commit
return self._proxied.commit()
^^^^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/session.py", line 2028, in commit
trans.commit(_to_root=True)
File "", line 2, in commit
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/state_changes.py", line 139, in _go
ret_value = fn(self, *arg, **kw)
^^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/session.py", line 1313, in commit
self._prepare_impl()
File "", line 2, in _prepare_impl
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/state_changes.py", line 139, in _go
ret_value = fn(self, *arg, **kw)
^^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/session.py", line 1288, in _prepare_impl
self.session.flush()
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/session.py", line 4352, in flush
self._flush(objects)
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/session.py", line 4487, in _flush
with util.safe_reraise():
^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/util/langhelpers.py", line 146, in exit
raise exc_value.with_traceback(exc_tb)
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/session.py", line 4448, in _flush
flush_context.execute()
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/unitofwork.py", line 466, in execute
rec.execute(self)
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/unitofwork.py", line 642, in execute
util.preloaded.orm_persistence.save_obj(
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/persistence.py", line 93, in save_obj
_emit_insert_statements(
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/orm/persistence.py", line 1233, in _emit_insert_statements
result = connection.execute(
^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1418, in execute
return meth(
^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/sql/elements.py", line 515, in _execute_on_connection
return connection._execute_clauseelement(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1640, in _execute_clauseelement
ret = self._execute_context(
^^^^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1846, in _execute_context
return self._exec_single_context(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1986, in _exec_single_context
self._handle_dbapi_exception(
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 2355, in _handle_dbapi_exception
raise sqlalchemy_exception.with_traceback(exc_info[2]) from e
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/engine/base.py", line 1967, in _exec_single_context
self.dialect.do_execute(
File "/home/hyi/dify/api/.venv/lib/python3.12/site-packages/sqlalchemy/engine/default.py", line 941, in do_execute
cursor.execute(statement, parameters)
sqlalchemy.exc.IntegrityError: (psycopg2.errors.NotNullViolation) null value in column "updated_at" of relation "workflows" violates not-null constraint
DETAIL: Failing row contains (8e7a5094-474e-4291-bfa1-403cecc81962, 835ec9f4-3fdc-4cde-bef0-adf492904099, f1b7ef1a-f232-44ac-aa42-ef2874aeb7fe, chat, draft, {"nodes": [{"id": "1734830783383", "type": "custom", "data": {"t..., {"retriever_resource": {"enabled": true}, "file_upload": {}, "op..., 84a19c38-a3fb-4bb2-824f-eb76eb80c74e, 2024-12-22 01:26:24, null, null, {}, {}).

[SQL: INSERT INTO workflows (tenant_id, app_id, type, version, graph, features, created_by, updated_by, environment_variables, conversation_variables) VALUES (%(tenant_id)s::UUID, %(app_id)s::UUID, %(type)s, %(version)s, %(graph)s, %(features)s, %(created_by)s::UUID, %(updated_by)s::UUID, %(environment_variables)s, %(conversation_variables)s) RETURNING workflows.id, workflows.created_at, workflows.updated_at]
[parameters: {'tenant_id': '835ec9f4-3fdc-4cde-bef0-adf492904099', 'app_id': 'f1b7ef1a-f232-44ac-aa42-ef2874aeb7fe', 'type': 'chat', 'version': 'draft', 'graph': '{"nodes": [{"id": "1734830783383", "type": "custom", "data": {"type": "start", "title": "Start", "desc": "", "variables": []}, "position": {"x": 80, ... (933 characters truncated) ... t": "llm", "targetHandle": "target"}, {"id": "llm-answer", "source": "llm", "sourceHandle": "source", "target": "answer", "targetHandle": "target"}]}', 'features': '{"retriever_resource": {"enabled": true}, "file_upload": {}, "opening_statement": "", "suggested_questions": [], "suggested_questions_after_answer": ... (23 characters truncated) ... eech_to_text": {"enabled": false}, "text_to_speech": {"enabled": false, "voice": "", "language": ""}, "sensitive_word_avoidance": {"enabled": false}}', 'created_by': '84a19c38-a3fb-4bb2-824f-eb76eb80c74e', 'updated_by': None, 'environment_variables': '{}', 'conversation_variables': '{}'}]
(Background on this error at: https://sqlalche.me/e/20/gkpj)

@laipz8200
Copy link
Member

Temporarily removed server_default in Workflow.updated_at in #11960 and added the correct default value for it.

@jiangbo721
Copy link
Contributor Author

Temporarily removed server_default in Workflow.updated_at in #11960 and added the correct default value for it.

I think the reason why this is going wrong is mainly due to data. Because nullable=False was not set before this commit e61752b, resulting in a potentially empty database.
Possible solutions:

  1. nullable=True
  2. Before the next migration execute SQL UPDATE workflows SET updated_at = created_at WHERE updated_at IS NULL, and then the code will be changed to
    updated_at: Mapped[datetime] = mapped_column(db.DateTime, nullable=False, server_default=func.current_timestamp(), server_onupdate=func.current_timestamp())

@laipz8200
Copy link
Member

In the database, the updated_at of the workflows table is set to NOT NULL, we can add a server_default and generate a new migration file to set the default value in the database. However, based on my tests, #11960 works well for this issue.

@yihong0618
Copy link
Contributor

both empty and not empty tested we need a default here

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🐞 bug Something isn't working lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants