Problem/Motivation

We need an upgrade path from 7.x for the core Syslog module.

Remaining Tasks

There's no real need to migrate any data from syslog. But any variables it maintains need to be moved into configuration.

Contributors

@-enzo- wrote the original migration, and @oadaeh uploaded it; @bdimaggio updated it for HEAD and wrote a test.

git commit -m 'Issue #2500529 by -enzo-, bdimaggio, oadaeh: Upgrade path for Syslog 7.x'

CommentFileSizeAuthor
#39 interdiff-2500529-37-39.txt1.68 KBphenaproxima
#39 2500529-39.patch3.09 KBphenaproxima
#37 interdiff-2500529-35-37.txt3.69 KBphenaproxima
#37 2500529-37.patch2.2 KBphenaproxima
#35 interdiff-2500529-33-35.txt1006 bytesphenaproxima
#35 2500529-35.patch2.18 KBphenaproxima
#33 interdiff-2500529-28-33.txt756 bytesphenaproxima
#33 2500529-33.patch2.39 KBphenaproxima
#28 2500529-28.patch2.4 KBphenaproxima
#26 interdiff-2500259-21-26.txt1.52 KBphenaproxima
#26 2500529-26.patch2.4 KBphenaproxima
#21 upgrade_path_for_syslog-2500529-21.patch2.5 KBbdimaggio
#18 upgrade_path_for_syslog-2500529-18.patch2.5 KBbdimaggio
#15 upgrade_path_for_syslog-2500529-15.patch2.56 KBbdimaggio
#4 upgrade_path_for_syslog-2500529-4.patch2.53 KBbdimaggio
#3 upgrade_path_for_syslog-2500529-3.patch2.57 KBbdimaggio
#2 migrate.migration.d7_syslog_settings.yml401 bytesbdimaggio

Comments

bdimaggio’s picture

bdimaggio
he/him
Location Boston, MA
commented
Assigned: Unassigned » bdimaggio
bdimaggio’s picture

bdimaggio
he/him
Location Boston, MA
commented
Assigned: bdimaggio » Unassigned
StatusFileSize
new migrate.migration.d7_syslog_settings.yml401 bytes

OK, got the YAML file together and verified that it works in a D7->D8 migration. Tests are on the way.

bdimaggio’s picture

bdimaggio
he/him
Location Boston, MA
commented
Assigned: Unassigned » bdimaggio
StatusFileSize
new upgrade_path_for_syslog-2500529-3.patch2.57 KB

Got test together. It fails with a fatal error, but so do the other, preexisting tests I've run (see e.g. MigrateActionConfigsTest and MigrateAggregatorConfigsTest). phenaproxima, do you think the problem is upstream from this work?

bdimaggio’s picture

bdimaggio
he/him
Location Boston, MA
commented
StatusFileSize
new upgrade_path_for_syslog-2500529-4.patch2.53 KB

Dammit. Left a bit of test code in.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review

Changing status for testbot.

The last submitted patch, 3: upgrade_path_for_syslog-2500529-3.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 4: upgrade_path_for_syslog-2500529-4.patch, failed testing.

Status: Needs work » Needs review

phenaproxima queued 4: upgrade_path_for_syslog-2500529-4.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: upgrade_path_for_syslog-2500529-4.patch, failed testing.

The last submitted patch, 4: upgrade_path_for_syslog-2500529-4.patch, failed testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Postponed

Postponing on #2510072: UTF-8 support in MySQL driver breaks migrate dump files, since tests will not work until that one is committed.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Postponed » Needs review

phenaproxima queued 4: upgrade_path_for_syslog-2500529-4.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: upgrade_path_for_syslog-2500529-4.patch, failed testing.

bdimaggio’s picture

bdimaggio
he/him
Location Boston, MA
commented
StatusFileSize
new upgrade_path_for_syslog-2500529-15.patch2.56 KB

OK, got tests working on my end. Crossing fingers...

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review

@bdimaggio -- to automatically test patches, be sure to set the status to "needs review". ;-)

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Needs work 
+++ b/core/modules/migrate_drupal/src/Tests/d7/MigrateSyslogConfigsTest.php
+/**
+ * @file
+ * Contains \Drupal\migrate_drupal\Tests\d6\MigrateAggregatorConfigsTest.
+ */

This needs to have the actual name of the test class. :)

+use Drupal\migrate_drupal\Tests\d7\MigrateDrupal7TestBase;

IIRC, MigrateDrupal7TestBase doesn't need to be use'd here, because it's in the same namespace.

+ * @group migrate_drupal

Can you make the group migrate_drupal 7.x (including the space), for consistency with the other tests in the D7 migration path?

Other than these things, it looks good to me!

bdimaggio’s picture

bdimaggio
he/him
Location Boston, MA
commented
StatusFileSize
new upgrade_path_for_syslog-2500529-18.patch2.5 KB

Rrrrright I could see how people would want the actual class name to be here :)

Fixed that, removed the "use" of MigrateDrupal7TestBase, and added 7.x to @group. How's this look?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review

Changing status for testbot.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Me likely. I declare this RTBC.

bdimaggio’s picture

bdimaggio
he/him
Location Boston, MA
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new upgrade_path_for_syslog-2500529-21.patch2.5 KB

I couldn't help looking at it again this morning, and realizing that I wasn't taking advantage of my own $modules property when enabling the syslog module in setUp(). That's fixed now and I will stop fiddling.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Looks okay to me.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes

Adding contributors and commit message to the IS.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
index 0000000..4ad4f5b
--- /dev/null

--- /dev/null
+++ b/core/modules/migrate_drupal/config/optional/migrate.migration.d7_syslog_settings.yml

+++ b/core/modules/migrate_drupal/config/optional/migrate.migration.d7_syslog_settings.yml
@@ -0,0 +1,21 @@

@@ -0,0 +1,21 @@
+id: d7_syslog_settings
+label: Drupal 7 syslog configuration
+migration_tags:
+  - Drupal 7
+source:
+  plugin: variable
+  variables:
+    - syslog_facility
+    - syslog_format
+    - syslog_identity
+process:
+  facility: syslog_facility
+  format: syslog_format
+  identity: syslog_identity
+destination:
+  plugin: config
+  config_name: syslog.settings
+dependencies:
+  module:
+    - syslog
+    - migrate_drupal

This needs to be moved to a template - see #2463909: Migrations should support non-installed default configurations (templates)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Also this issue needs to be updated for #2495271: Split the migrate_drupal test suite in to 6.x and 7.x groups

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 2500529-26.patch2.4 KB
new interdiff-2500259-21-26.txt1.52 KB

Re-rolled.

Status: Needs review » Needs work

The last submitted patch, 26: 2500529-26.patch, failed testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 2500529-28.patch2.4 KB

Changed test group.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes

Added @-enzo- as a contributor, since this migration is originally based on his work in #2382117: Migration Files for Drupal 7 Variables.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes
benjy’s picture

benjy commented
Status: Needs review » Reviewed & tested by the community

Looks good.

benjy’s picture

benjy commented 
+++ b/core/modules/migrate_drupal/src/Tests/d7/MigrateSyslogConfigsTest.php
@@ -0,0 +1,55 @@
+    $this->installConfig(self::$modules);

This should be static::

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2500529-33.patch2.39 KB
new interdiff-2500529-28-33.txt756 bytes

Corrected.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

@benjy re-RTBCed by way of me. :)

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new 2500529-35.patch2.18 KB
new interdiff-2500529-33-35.txt1006 bytes

Re-rolled for #2514168: Streamline migrate_drupal integration tests.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Upgrade path for Syslog 7.x » Migration path for Syslog 7.x

Title was tripping me out.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2500529-37.patch2.2 KB
new interdiff-2500529-35-37.txt3.69 KB

Re-rolled in accordance with #2533886: [meta] Move module-specific migration support into the particular modules supported.

chx’s picture

chx commented
Title: Migration path for Syslog 7.x » Migration path for Syslog 7.x configuration
Priority: Minor » Normal
Status: Needs review » Needs work

We need to set facility in the source so we can actually test it's being migrated. Asserting an empty string doesn't tell us much.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 2500529-39.patch3.09 KB
new interdiff-2500529-37-39.txt1.68 KB

Fixed.

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

This is a go for now and @phenaproxima will open a bug report because facility is an int. http://man7.org/linux/man-pages/man3/syslog.3.html void openlog(const char *ident, int option, int facility);

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

It's nice since migration code in the module that owns the data. Migrate is not subject to beta evaluation. Committed 97ded67 and pushed to 8.0.x. Thanks!

  • alexpott committed 97ded67 on 8.0.x 
    Issue #2500529 by phenaproxima, bdimaggio: Migration path for Syslog 7.x...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.