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.

Sign up for GitHub

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

Jump to bottom

ES-7: remove type #1666

Merged
merged 29 commits into from Oct 31, 2019
Merged

ES-7: remove type #1666

merged 29 commits into from Oct 31, 2019

Conversation

thePanz
Copy link
Collaborator

@thePanz thePanz commented Oct 3, 2019 •
edited

Todo:

@thePanz
Copy link
Collaborator Author

thePanz commented Oct 3, 2019

@ruflin the usage of include_type_name was too hard to remove while keeping a backward-compatibility. This PR is a first step to removing the Type completely, and moving its functionalities into the Index class.

@thePanz thePanz changed the title WIP: Es 7 remove type WIP: ES-7 remove type Oct 3, 2019
@renanbr
Copy link
Contributor

renanbr commented Oct 7, 2019

Assuming everything is fine in src/ I changed some tests.
Let me know if it's in the good direction.

Changes I made, but not 100% convinced:

  • Elastica\Test\BulkTest::testSend() and Elastica\Test\BulkTest::testRawDocumentDataRequest()
    • orpahend document (has no index) is always tied to the type "_doc"

Wouldn't we deprecate (and auto-redirect when object is given) also these methods ?

  • Elastica\Bulk->setType() ==> Elastica\Bulk->setIndex()
  • Elastica\Search->addType() ==> Elastica\Search->addIndex()
  • Elastica\Search->addTypes() ==> Elastica\Search->addIndices()

Additional note:

  • Elastica\Client::deleteIds() and Elastica\Client::updateDocument() still require $type argument, is this intentional?

@thePanz
Copy link
Collaborator Author

thePanz commented Oct 7, 2019

@renanbr thank you for your findings, I updated the PR description

@renanbr
Copy link
Contributor

renanbr commented Oct 10, 2019

@thePanz I've submitted some PRs:

Let me know if they are in the good direction.

The missing tasks are:

  • Remore Type class
  • Fix tests*

* tests for these classes I've changed are green :)

@thePanz
Copy link
Collaborator Author

thePanz commented Oct 10, 2019 •
edited

@renanbr I'll have a look asap.

I think that all your changes should be merged into this MR, to have all "remove Type class" changes in one single pull-request. wdyt?

update my bad, all your PR are targeting this branch :)

@thePanz thePanz force-pushed the es-7-remove-type branch 17 times, most recently from f40b9db to 3c51f0b Compare October 10, 2019 15:59
alamirault
alamirault reviewed Oct 10, 2019
View reviewed changes
lib/Elastica/Index.php Outdated Show resolved Hide resolved
lib/Elastica/Index.php Outdated Show resolved Hide resolved
lib/Elastica/Search.php Show resolved Hide resolved
@thePanz thePanz force-pushed the es-7-remove-type branch 2 times, most recently from 9ce86e9 to 7aa1778 Compare October 11, 2019 08:50
@renanbr
Copy link
Contributor

renanbr commented Oct 11, 2019

more test fixes #1674 (targeting this branch, as usual)

@renanbr
Copy link
Contributor

renanbr commented Oct 14, 2019

Tests are green 😄
Is there still a task to be done before getting this PR merged?
Is there a plan to release a major version compatible with ES 7?

@ruflin
Copy link
Owner

ruflin commented Oct 14, 2019

@renanbr Main thing missing is me getting back from PTO and catching up on things. 7.0 release is planned yes. Hope to get all these in this and next week 🤞

@renanbr
Copy link
Contributor

renanbr commented Oct 14, 2019

Thanks for the quick response! Let me know if you need some more hand 😉

thePanz and others added 15 commits October 30, 2019 17:34
@thePanz
Copy link
Collaborator Author

thePanz commented Oct 30, 2019 •
edited

@ruflin rebased and pushed!
I agree with having a 7.0.0-beta1 tag, that's would be great!

@thePanz thePanz self-assigned this Oct 30, 2019
@thePanz thePanz mentioned this pull request Oct 30, 2019
Closed
@thePanz thePanz changed the title WIP: ES-7 remove type ES-7: remove type Oct 30, 2019
@ruflin ruflin merged commit 9979894 into master Oct 31, 2019
@ruflin ruflin deleted the es-7-remove-type branch October 31, 2019 07:31
@ruflin
Copy link
Owner

ruflin commented Oct 31, 2019

Merged 🎉 🎈

@ruflin
Copy link
Owner

ruflin commented Oct 31, 2019

Just created #1685 to prepare the 7.0.0-beta1 release.

@deguif deguif mentioned this pull request Nov 1, 2019
Closed
tmotyl added a commit to macopedia/elasticorn that referenced this pull request Mar 30, 2021
@tmotyl
Update Compatibility with Elasticsearch 7
b5b873b 
* bump Elastica version
* migrate away from Type\Mapping & Index has no getType method any more see:
ruflin/Elastica#1666
* FYI IndexConfiguration.yaml should now have following structure:

settings:
  number_of_shards: 4
  number_of_replicas: 1

instead of just

number_of_shards: 4
number_of_replicas: 1

* Use real index name instead of alias when adding new alias.
@tmotyl tmotyl mentioned this pull request Mar 30, 2021
Merged
tmotyl added a commit to macopedia/elasticorn that referenced this pull request Mar 30, 2021
@tmotyl
Update Compatibility with Elasticsearch 7
8da216f 
* bump Elastica version
* migrate away from Type\Mapping & Index has no getType method any more see:
ruflin/Elastica#1666
* Use real index name instead of alias when adding new alias.
susannemoog pushed a commit to TYPO3GmbH/elasticorn that referenced this pull request Apr 2, 2021
@tmotyl @susannemoog
Update Compatibility with Elasticsearch 7
7e28763 
* bump Elastica version
* migrate away from Type\Mapping & Index has no getType method any more see:
ruflin/Elastica#1666
* Use real index name instead of alias when adding new alias.
susannemoog added a commit to TYPO3GmbH/elasticorn that referenced this pull request Apr 17, 2021
@tmotyl @susannemoog
Update Compatibility with Elasticsearch 7 (#17)
a17d0eb 
* Update Compatibility with Elasticsearch 7


* bump Elastica version
* migrate away from Type\Mapping & Index has no getType method any more see:
ruflin/Elastica#1666
* Use real index name instead of alias when adding new alias.

* Rebase and update dependencies

Co-authored-by: Susanne Moog <look@susi.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alamirault alamirault alamirault left review comments

@philkra philkra philkra left review comments

@ruflin ruflin ruflin approved these changes

Assignees

@thePanz thePanz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@thePanz @renanbr @ruflin @philkra @alamirault