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

bug(bridge): Doctrine::update() #45

Closed
jvancoillie opened this issue Mar 30, 2021 · 14 comments · Fixed by #46
Closed

bug(bridge): Doctrine::update() #45

jvancoillie opened this issue Mar 30, 2021 · 14 comments · Fixed by #46
Assignees
Labels
Bridge bug Something isn't working PHP

Comments

@jvancoillie
Copy link
Contributor

Hi,

Task update fails due to lock mode (getWriteLockSQL()) which I think is only available on a SQL SELECT command.

that throws the following exception

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'FOR UPDATE' at line 1
@Guikingone
Copy link
Owner

Guikingone commented Mar 30, 2021

Hi @jvancoillie 👋🏻

Hum, strange behaviour here 🤔

This method is tested on SQLite and seems to work, do you have more informations about the stack used? Maybe the SGBDR driver and so on?

Thanks and sorry for the issue 🙂

@Guikingone Guikingone self-assigned this Mar 30, 2021
@Guikingone Guikingone added bug Something isn't working PHP Bridge labels Mar 30, 2021
@Guikingone Guikingone added this to the Current (as patch) milestone Mar 30, 2021
@Guikingone Guikingone changed the title Doctrine update task bug(bridge): Doctrine::update() Mar 30, 2021
@Guikingone Guikingone linked a pull request Mar 30, 2021 that will close this issue
@jvancoillie
Copy link
Contributor Author

no problem, I discover the bundle and try to understand how it works so that I can use it on a current project.

It is possible that the problem does not come from the bundle but from the use that I make of it.

Currently I am on version 5.2.6 of symfony with php 7.4.15 and a mariadb in version 10.5.8 (docker mariadb:10.5.8) with the following overrided configuration:

[mysqld]
sql-mode = "STRICT_TRANS_TABLES, NO_ZERO_IN_DATE, ERROR_FOR_DIVISION_BY_ZERO, NO_ENGINE_SUBSTITUTION"
character-set-server = utf8
default-authentication-plugin = mysql_native_password

@Guikingone
Copy link
Owner

Hum, ok, I got a better understanding of the problem, it seems that when executing SQLite queries, the driver returns an empty string for this method, this way, we cannot grab the error, not to mention that you're right about the usage of it ONLY in SELECT statements (shame on me 😄 ).

I've opened a PR to fix the issue, should be out soon in 0.3.5.

@jvancoillie
Copy link
Contributor Author

jvancoillie commented Mar 30, 2021

I tested without the writeLockSQL and when updating the task it does not detect any change which throws the exception "The given task cannot be updated as the identifier or the body is invalid" and rollback because the rowCount ( ) equal to 0.

[2021-03-30T16:55:45.811072+00:00] doctrine.DEBUG: "START TRANSACTION" [] []
[2021-03-30T16:55:45.811819+00:00] doctrine.DEBUG: UPDATE scheduler_tasks SET body = :body WHERE task_name = :name {":name":"activation",":body":"{\"body\":{\"command\":\"app:ac [...]"} []
[2021-03-30T16:55:45.812709+00:00] doctrine.DEBUG: "ROLLBACK" [] []

sorry for the inconvenience i really must do something wrong

@Guikingone
Copy link
Owner

Do you have the whole process that lead to this error? 🤔

It is during a specific command?

@jvancoillie
Copy link
Contributor Author

jvancoillie commented Mar 30, 2021

yes and sorry in advance for the screenshot. here is the complete process
Capture d’écran 2021-03-30 à 19 01 50

@jvancoillie
Copy link
Contributor Author

is it because of the "single_run" parameter i see it removes the entry and then tries to do an update

@Guikingone
Copy link
Owner

Oh, that's what I call a great issue 😄

Ok, got it, by default and if single_run is set to true, the task is unscheduled right after the execution, thing is, the process of updating the task is not aborted and the connection cannot do the update 🤔

Looks like a "human-related" bug on my side 😄

@jvancoillie
Copy link
Contributor Author

Must understand that the single_run parameter, will only execute the command once ?

In any case this is the behavior I expect, I want to play the "app:activation" command when setting up the cron bin/console scheduler:consume which will have the effect of activating a parameter for startup of the application and which will never be played again on future calls to the consume command.
I will have other commands later that will be played @monthly

Désolé pour mon anglais médiocre ;)

@Guikingone
Copy link
Owner

Guikingone commented Mar 30, 2021

Désolé pour mon anglais médiocre ;)

Aucun souci pour l'anglais, le mien n'est pas meilleur (et je ne parle pas de l'accent made in "Chud de la France" 😄 ).

The single_run is made to execute the task once and only once, BTW, you create an interesting use case where we can have tasks that must be executed only once when deploying / booting the scheduler / etc.

Could be interesting to think about it for next releases 🙂

@jvancoillie
Copy link
Contributor Author

Great, what is a shame is that I will no longer have information on the single_run command. Currently, when I list the scheduler I find my activation command (certainly because of the bug).

Capture d’écran 2021-03-30 à 19 40 00

After the correction, I should not have any more information about this task, is that right?

it could have been interesting to be able to consult the execution time of this command and to keep track of the last execution

@Guikingone
Copy link
Owner

Guikingone commented Mar 30, 2021

Hum, in fact, the behaviour can be changed / improved, I'm thinking about the fact that a single_run task can be disabled or paused once executed rather than removed from the scheduler 🤔

@Guikingone
Copy link
Owner

Small update, the fix has been merged, as the core API has been changed (unschedule to pause), the fix is moved to 0.4 for release 🙂

@jvancoillie
Copy link
Contributor Author

@Guikingone thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bridge bug Something isn't working PHP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants