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

Neuer Modus FORCE_FILES #66

Merged
merged 1 commit into from
Feb 12, 2018
Merged

Neuer Modus FORCE_FILES #66

merged 1 commit into from
Feb 12, 2018

Conversation

gharlan
Copy link
Member

@gharlan gharlan commented Feb 11, 2018

Bisher gibt es zwei Modi für die Synchronisation.
Der normale Modus, wo je nach Zeitstempel der DB-Datensatz oder die Datei gewinnt.
Und dann gibt es den force-Mode, wo immer die DB gewinnt. Dieser wird zum Beispiel nach einem Backup-Import ausgeführt.

Für YDeploy benötigt ich aber das Gegenteil, einen force-Mode, wo die Dateien gewinnen: yakamara/ydeploy#7
Beim Deployen soll der per Dateien aufgespielte Stand garantiert werden.

Daher schlage ich vor, den force-Mode zu trennen in FORCE_DB und FORCE_FILES.
YDeploy würde dann redaxo/bin/console developer:sync --force-files nutzen.

lib/command.php Outdated
@@ -10,6 +12,8 @@ protected function configure()
$this
->setName('developer:sync')
->setDescription('Synchronizes the developer files')
->addOption('force-db', null, InputOption::VALUE_NONE, 'Force the current status in db')
Copy link
Member

Choose a reason for hiding this comment

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

, files will be overridden

lib/command.php Outdated
@@ -10,6 +12,8 @@ protected function configure()
$this
->setName('developer:sync')
->setDescription('Synchronizes the developer files')
->addOption('force-db', null, InputOption::VALUE_NONE, 'Force the current status in db')
->addOption('force-files', null, InputOption::VALUE_NONE, 'Force the current status in file system')
Copy link
Member

Choose a reason for hiding this comment

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

db data will be overridden

/**
* The method is called, when an existing item is deleted by the file system (`FORCE_FILES` is activated)
*
* Use the method to delete the item in the base system
Copy link
Member

@staabm staabm Feb 11, 2018

Choose a reason for hiding this comment

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

Base system?

Copy link
Member Author

Choose a reason for hiding this comment

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

Kommt per copy/paste von den anderen Methoden.

Das Ding ist, das developer-Addon ist nicht nur darauf ausgelegt, in die DB zu synchronisieren, sondern theoretisch sonst wo hin. Also die eine Seite sind immer Dateien (Developer-Dateien). Die andere Seite bestimmen die Synchronizer. Die drei default Synchronizer (Templates, Module, Actions) syncen halt in die DB. Diese Basis-Synchronizer-Klasse lässt das aber offen.

*/
public static function start()
public static function start($force = false)
Copy link
Member

Choose a reason for hiding this comment

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

willste nicht lieber nur noch die konstanten unterstützen statt dem bool?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ich will keinen BC-Break..
false == kein force
1 oder true == force_db (so wie auch vorher bei true)
2 == force_files

Copy link
Member

@staabm staabm left a comment

Choose a reason for hiding this comment

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

Ungetested

@staabm
Copy link
Member

staabm commented Feb 11, 2018

Ggf wäre es sinnvoll diesen mode generell für eine redaxo instanz festzulegen statt nur für einen commando aufruf...?

Aktuell stehst du in meinen augen vor dem problem dass du ggf. Jmd anderem seine arbeit überschreibst, die via webinterface gemacht wurde (oder umgekehrt).

Wenn man den mode in der instanz einstellt könnte man in der redaxo oberfläche das editieren sperren/eine warnung ausgeben, wenn via files gearbeiten werden soll/muss etc

@gharlan
Copy link
Member Author

gharlan commented Feb 12, 2018

Aktuell stehst du in meinen augen vor dem problem dass du ggf. Jmd anderem seine arbeit überschreibst, die via webinterface gemacht wurde (oder umgekehrt).

Konkret in meinem/unseren Use case besteht das Problem nicht. Auf den Servern ist das Syncing im Backend und Frontend komplett ausgeschaltet, wird immer nur einmalig beim Deployen gesynct.
Im Backend blenden wir da auch bereits die Module- und Template-Verwaltung aus.

Ganz allgemein magst du eventuell Recht haben. Finde es aber trotzdem gut, im Kommando diese Force-Options zu haben, und würde es daher erst mal so belassen, da uns das so erstmal reicht.
Kann man ggf. wann anders aber so erweitern, dass der Default-Modus eingestellt werden kann.

@gharlan gharlan merged commit 70b0249 into FriendsOfREDAXO:master Feb 12, 2018
@gharlan gharlan deleted the force-files branch February 12, 2018 23:16
@staabm
Copy link
Member

staabm commented Feb 13, 2018

Konkret in meinem/unseren Use case besteht das Problem nicht. Auf den Servern ist das Syncing im Backend und Frontend komplett ausgeschaltet, wird immer nur einmalig beim Deployen gesynct.

vllt würde es sinn machen ein initiales setup commando mit ydeploy zu haben, was einem diese einstellung vorschlägt?
generell wäre vermutlich ein setup commando gut um die "remote-infos" vom user interaktiv zusammenzutragen, wenn sie noch nicht vorhanden sind?

@gharlan
Copy link
Member Author

gharlan commented Feb 13, 2018

vllt würde es sinn machen ein initiales setup commando mit ydeploy zu haben, was einem diese einstellung vorschlägt?
generell wäre vermutlich ein setup commando gut um die "remote-infos" vom user interaktiv zusammenzutragen, wenn sie noch nicht vorhanden sind?

Ja, ist geplant.
yakamara/yak#7
yakamara/ydeploy#11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants