Skip to content

feat: Add 4 Aurora engine-behavior skills (parameter, replication, upgrade, lifecycle) - #58

Open
kiranmam wants to merge 1 commit into
aws:mainfrom
kiranmam:feat/aurora-engine-behavior-skills
Open

feat: Add 4 Aurora engine-behavior skills (parameter, replication, upgrade, lifecycle)#58
kiranmam wants to merge 1 commit into
aws:mainfrom
kiranmam:feat/aurora-engine-behavior-skills

Conversation

@kiranmam

Copy link
Copy Markdown
Contributor

Four standalone skills covering ~15,000 cases/yr of Aurora MySQL + PostgreSQL engine-behavior gaps. All use AWS CLI + CloudWatch only — no MCP, no Data API. Covers parameter-group misconfigs (#14/#151), replication health (#135), upgrade readiness (#37/#137), and lifecycle/right-sizing (#42/#34).

@chetanandikanti

Copy link
Copy Markdown

Review — Aurora engine-behavior skills (4 skills)

Reviewed at PR head b37839b. Technically these are strong — the engine behavior is accurate (Aurora MySQL auto-manages innodb_buffer_pool_size; PostgreSQL shared_buffers = {DBInstanceClassMemory/32768} ≈ 25%; in-region storage-level replication is typically sub-second; there is no direct Serverless v1→v2 path; optimizer statistics reset requires a post-upgrade ANALYZE). Every skill's scoring dimensions sum to 100, the skills cross-reference each other cleanly, and the upgrade-advisor honestly flags the in-database checks it can't perform control-plane-only. A few items before this can merge:

Blocking

  1. Required files missing (all 4 skills). Each ships SKILL.md only. Per the Agent Skills specification and CONTRIBUTING, every skill needs a README.md (with the non-production disclaimer and IAM prerequisites — rds:Describe*, cloudwatch:GetMetricStatistics, application-autoscaling:*, relevant ec2:Describe*), a CHANGELOG.md, and an evals/ directory (evals.json + eval_queries.json) with a passing eval run.
  2. Frontmatter. All four use top-level version: / author:. Nest version and author under a metadata: block per the spec, and use the author alias (e.g. kiranmam) to match the merged database-rds-devops skill.
  3. **Unclosed yaml fence (all 4).** `## DETECTION RULES` opens a ` yaml block that is never closed — verified: each file contains exactly one fence marker and zero closing fence. As a resultASSESSMENT COMMANDS, ASSESSMENT SCORING MATRIX, REMEDIATION PLAYBOOK TEMPLATES, and REPORT OUTPUT FORMATall render inside one code block and lose their##` headings. Close the fence and restore the headings in every file.

Content / consistency

  • database-aurora-lifecycle-advisor: PART A … (#42) / PART B … (#34) and RS-05 "matches Aadd reference to demo video on The Keys to AWS Optimization #34 pattern" — in this repo #42 / #34 render as links to PRs/issues. These read as stray internal references; please strip them.
  • database-aurora-replication-health: RL-01 describes lag as "elevated (>20 ms sustained; >100 ms = WARNING)," but the Thresholds and detection-rule sections use >100 ms = WARNING and >1000 ms = CRITICAL. Reconcile the "elevated" threshold.

Structure / scope

  • Files are 443 / 411 / 365 / 381 lines — under the 500-line guideline but heavy; consider moving the catalogs and playbook templates into references/ (see eks-operation-review).
  • Four skills in one PR — each still needs its own README / CHANGELOG / evals. Consider one PR per skill, or one PR with all four fully packaged. Also worth confirming distinctness vs the merged database-rds-devops skill.

Happy to re-review once these are addressed. Thanks for the strong technical content here.

@chetanandikanti

Copy link
Copy Markdown

Addendum — how this was reviewed (methodology, reasoning, next steps)

Steps

  1. Fetched the PR head (git fetch origin pull/58/head) and diffed against main — confirmed the change set is exactly four new SKILL.md files.
  2. Read all four skills in full and checked the engine-behavior claims against known Aurora behavior.
  3. Ran objective checks instead of eyeballing: counted triple-backtick fences per file (each has 1 opening / 0 closing → unclosed block), counted lines (443 / 411 / 365 / 381), and listed each skill directory (only SKILL.md present).
  4. Verified each skill's four scoring dimensions arithmetically sum to 100.
  5. Cross-checked the two internal-consistency items (lifecycle #42 / #34; replication-health RL-01 threshold vs the Thresholds table).

Reasoning — why these are blocking

  • Missing README / CHANGELOG / evals and the frontmatter shape are hard requirements in the Agent Skills specification and CONTRIBUTING; review/CI will reject without them.
  • The unclosed fence isn't cosmetic — it hides the operational half of each skill (assessment commands, scoring matrix, playbooks, report format) inside one code block, so both a human reader and the agent's own section parsing lose those ## sections.
  • The two content items are correctness bugs (a stray in-repo issue link, and a threshold that contradicts the thresholds table) that would mislead users and the agent.

Recommended next steps (fastest path to green)

  1. Close the ```yaml fence and restore the ## headings in all four files (quick fix; immediately restores readability).
  2. Add README / CHANGELOG / evals per skill and run the eval to a pass.
  3. Fix frontmatter — nest version/author under metadata: and use the author alias.
  4. Fix the two content items (lifecycle stray refs; replication threshold).
  5. Consider splitting into one PR per skill (or fully package all four).

Re-ping me once addressed and I'll re-review.

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