Add Chewy 7/ES 7 to Chewy 8/ES 8 migration guide#1010
Add Chewy 7/ES 7 to Chewy 8/ES 8 migration guide#1010mattmenefee wants to merge 3 commits intotoptal:masterfrom
Conversation
mattmenefee
changed the title
mattmenefee
force-pushed
the
chewy-v8-migration-guide
branch
from
2fc70fc to
c2d6944
Compare
bbatsov
left a comment
bbatsov
left a comment
There was a problem hiding this comment.
The migration guide itself is solid and useful content, nice work. A few things to address:
- Changelog should use
## master (unreleased)instead of## 8.0.2 (unreleased), and drop the empty sections. Version numbers are decided at release time. - The punctuation normalization across the older migration sections inflates the diff and makes the actual changes harder to review. I'd either drop it or at least put it in its own commit.
- Speaking of which -- please split this into separate commits for each logical change: the new migration guide section, the namespace fixes in docs/YARD, the broken link fix, and any punctuation cleanup. Makes it much easier to review and bisect later.
migration_guide.md
Outdated
| In order to upgrade Chewy 7/Elasticsearch 7 to Chewy 8/Elasticsearch 8 in the most seamless manner you have to: | ||
|
|
||
| * Upgrade to the latest 7.x stable release (Chewy 7.6.0, Elasticsearch 7.17) | ||
| * Study carefully [Breaking changes in 8.0](https://www.elastic.co/guide/en/elasticsearch/reference/8.17/breaking-changes-8.0.html), make sure your application conforms. |
There was a problem hiding this comment.
This URL returns a 404, could you use the right URL?
There was a problem hiding this comment.
Sorry about that! I assumed that versions prior to v9.x had the same breaking changes page 🤦♂️
| * Upgrade to Chewy 8. | ||
| * Perform a [rolling upgrade](https://www.elastic.co/guide/en/elasticsearch/reference/8.17/rolling-upgrades.html) of Elasticsearch. | ||
|
|
||
| ## Chewy 6/Elasticsearch 6 to Chewy 7/Elasticsearch 7 |
There was a problem hiding this comment.
I think it is better to revert all changes below this line. The important part is the v8 migration.
There was a problem hiding this comment.
I reverted most of the changes. I think it's important to keep the fix for the broken rolling upgrade link though.
The elasticsearch-ruby v8 gem reorganized its module hierarchy: - The transport client moved from Elasticsearch::Transport::Client to Elasticsearch::Client - Transport errors moved from Elasticsearch::Transport::Transport::Errors to Elastic::Transport::Transport::Errors Update documentation and YARD annotations to reflect these changes.
mattmenefee
force-pushed
the
chewy-v8-migration-guide
branch
from
c2d6944 to
6d1f0ea
Compare
The migration guide previously only covered up to the Chewy 6→7 upgrade path. This adds a comprehensive section for users upgrading to Chewy 8 / Elasticsearch 8, covering: - Minimum Ruby (3.2) and Rails (7.2) version requirements - elasticsearch gem version bump (>= 8.14, < 9.0) - Namespace changes from elasticsearch-transport to elastic-transport - Transport logger/tracer configuration changes - Chewy.massacre / delete_all restriction due to ES 8 defaults - Elasticsearch 8 security-by-default considerations - Recommended rolling upgrade procedure Closes toptal#1004
mattmenefee
force-pushed
the
chewy-v8-migration-guide
branch
from
6d1f0ea to
5a9ae2f
Compare
mattmenefee
left a comment
mattmenefee
left a comment
There was a problem hiding this comment.
@bbatsov I corrected the changelog attribution and split this into 2 commits - is that sufficient or should I move the broken link fix to a separate commit as well?
migration_guide.md
Outdated
| In order to upgrade Chewy 7/Elasticsearch 7 to Chewy 8/Elasticsearch 8 in the most seamless manner you have to: | ||
|
|
||
| * Upgrade to the latest 7.x stable release (Chewy 7.6.0, Elasticsearch 7.17) | ||
| * Study carefully [Breaking changes in 8.0](https://www.elastic.co/guide/en/elasticsearch/reference/8.17/breaking-changes-8.0.html), make sure your application conforms. |
There was a problem hiding this comment.
Sorry about that! I assumed that versions prior to v9.x had the same breaking changes page 🤦♂️
| * Upgrade to Chewy 8. | ||
| * Perform a [rolling upgrade](https://www.elastic.co/guide/en/elasticsearch/reference/8.17/rolling-upgrades.html) of Elasticsearch. | ||
|
|
||
| ## Chewy 6/Elasticsearch 6 to Chewy 7/Elasticsearch 7 |
There was a problem hiding this comment.
I reverted most of the changes. I think it's important to keep the fix for the broken rolling upgrade link though.
| @@ -1,5 +1,11 @@ | |||
| # Changelog | |||
|
|
|||
| ## master (unreleased) | |||
There was a problem hiding this comment.
@bbatsov I'm happy to remove the empty sections here, but note that this is inconsistent with most other changelog entries below
PR toptal#1003 added Ruby 4.0 support but the README still listed 3.2-3.4. Bump the documented range to 3.2-4.0 to match.
3 participants
Summary
migration_guide.mdcovering Ruby/Rails version requirements, error class namespace changes, transport logger/tracer changes,Chewy.massacrerestrictions, and ES 8 security defaultsElasticsearch::Transport::*namespace references in docs and YARD annotations to match the ES v8+elastic-transportgem — code examples would fail if copied verbatimCloses #1004
Test plan
migration_guide.mdresolve correctly