konlue commented on PR #7677:
URL: https://github.com/apache/shenyu/pull/7677#issuecomment-6229680004
Hi <mark>@Aias00</mark>, thank you for the thorough review — the `RuleData`
wire-format catch in particular was exactly the edge I missed. All three items
are addressed:
1. **`RuleData` sync-contract compatibility**
- `RuleData.logged` now carries `@SerializedName(value = "logged",
alternate = "loged")` — the same mechanism as `RateLimiterHandle`, and Gson is
the deserializer on both sync paths.
- `AbstractShenyuPlugin.printLog` now uses
`Boolean.TRUE.equals(ruleData.getLogged())` as defence in depth.
- The release note states the required upgrade order explicitly: “upgrade
all bootstrap instances before, or together with, admin”, since the
new-admin/old-bootstrap direction cannot be tolerated in code.
- Red-green verified both ways:
- `testDeserializationAcceptsLegacyLogedKey` fails without the
alternate when parsing legacy `{"loged": true}` sync JSON.
- `printLogToleratesNullLoggedFlag` throws NPE without the guard when
invoking `printLog` with a null flag.
2. **Upgrade scripts gated on real engines**
- `native-sql-matrix.py` now executes both scripts in sequence on top of
the pinned v2.7.1 baseline:
`MIGRATIONS_FROM_BASELINE = ("2.7.1-upgrade-2.7.2",
"2.7.2-upgrade-2.7.3")`, keeping abort-on-SQL-error.
- The `native-sql` jobs on this PR will therefore run the `ALTER TABLE
... RENAME COLUMN` statements on all five engines.
- The migration files are read from the PR tree, so future additions only
need one list entry.
- The matrix’s post-migration assertions only read `plugin` columns and
table `id`s, so the renamed `selector`/`rule` columns do not disturb them.
3. **Upgrade guide**
- `db/upgrade/upgrade-guide.md` gained a “loged column renamed to logged
in 2.7.3” section listing all five scripts as mandatory.
- It includes the upgrade-order note and the stop-admin/run/restart
sequence mirroring the 2.7.2 section.
**Minor:** the matching one-word change in the dashboard source is ready as
https://github.com/apache/shenyu-dashboard/pull/723 (3 files, 18 occurrences:
`Selector.js`, `Rule.js`, `ToolsModal.js`).
**Verification:** `shenyu-common` + `shenyu-plugin-base` reactor tests
250/250 green with checkstyle. I agree the previous e2e red was the
pre-existing `DividePluginTest` flake; I’ll watch the new run and re-trigger if
it recurs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
