Skip to content

Conversation

@alallema
Copy link
Contributor

@alallema alallema commented May 27, 2021

Description
Start of SDK go modification:

  • Rewriting of the Client that implements this method:
    • Index(uid string)
    • GetIndex(indexID string)
    • GetAllIndexes()
    • CreateIndex(config IndexConfig)
    • GetOrCreateIndex(config IndexConfig)
    • GetKeys()
    • GetAllStats()
    • CreateDump()
    • GetDumpStatus(dumpUID string)
    • Version()
    • GetVersion()
    • Health()
    • IsHealthy()
    • DeleteIndex(uid string)
  • Rewriting of the Index that implements this method:
    • FetchInfo()
    • FetchPrimaryKey()
    • ChangePrimaryKey()
    • Delete()
    • AddDocuments()
    • UpdateDocuments()
    • GetDocument()
    • GetDocuments()
    • DeleteOneDocument()
    • DeletesDocuments()
    • DeleteAllDocuments()
    • GetAllUpdateStatus()
    • GetUpdateStatus()
    • Search()
    • GetSettings()
    • UpdateSettings()
    • ResetSettings()
    • GetStats()
    • WaitForPendingUpdate()
    • DefaultWaitForPendingUpdate()
  • Removing of intermediates interfaces
  • Rewriting of sample and Readme for the new usage
  • Add tests for every method

To do

  • Client
    • Rewrite Client
    • Add Test to Client
  • Index
    • Rewrite Index
    • Add Tests to Index
    • Adding test for WaitForPendingUpdate() Issue related
    • Rewrite Document
    • Add Tests to Document
    • Rewrite Search
    • Add Tests Search
    • Rewrite Settings
    • Add Tests Settings

Bug
Wrong accepted status code on /health (Health() method) #155

@alallema alallema force-pushed the rewrite-client branch 5 times, most recently from 4486082 to 390bb39 Compare May 31, 2021 11:45
This was linked to issues Jun 1, 2021
@alallema alallema force-pushed the rewrite-client branch 3 times, most recently from 15b074c to cd17447 Compare June 7, 2021 15:30
Copy link
Contributor

@eskombro eskombro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @alallema !

I really think this is improving readability and usability at the same time. The general structure is simplified, without loosing any functionality and making it more friendly. I made a few comments on things that were not clear to me or that I think could be improved, LMKWYT :)

Good job 🎉

eskombro
eskombro previously approved these changes Jun 15, 2021
Copy link
Contributor

@eskombro eskombro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@alallema alallema marked this pull request as ready for review June 15, 2021 10:12
@alallema alallema requested a review from eskombro June 15, 2021 11:23
@alallema alallema linked an issue Jun 15, 2021 that may be closed by this pull request
@alallema alallema added the breaking-change The related changes are breaking for the users label Jun 17, 2021
@alallema alallema linked an issue Jun 17, 2021 that may be closed by this pull request
Copy link
Contributor

@eskombro eskombro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything LGTM but from my POV there is a little mistake on the README, can you check?

🎉

@alallema alallema requested a review from eskombro June 22, 2021 08:10
Copy link
Contributor

@eskombro eskombro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! 🎉

@alallema
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Jun 22, 2021
150: Rewrite client r=alallema a=alallema

**Description**
Start of SDK go modification:
- Rewriting of the Client that implements this method:
	- Index(uid string)
	- GetIndex(indexID string)
	- GetAllIndexes()
	- CreateIndex(config IndexConfig)
	- GetOrCreateIndex(config IndexConfig)
	- GetKeys()
	- GetAllStats()
	- CreateDump()
	- GetDumpStatus(dumpUID string)
	- Version()
	- GetVersion()
	- Health()
	- IsHealthy()
	- DeleteIndex(uid string)
- Rewriting of the Index that implements this method:
	- FetchInfo()
	- FetchPrimaryKey()
	- ChangePrimaryKey()
	- Delete()
	- AddDocuments()
	- UpdateDocuments()
	- GetDocument()
	- GetDocuments()
	- DeleteOneDocument()
	- DeletesDocuments()
	- DeleteAllDocuments()
	- GetAllUpdateStatus()
	- GetUpdateStatus()
	- Search()
	- GetSettings()
	- UpdateSettings()
	- ResetSettings()
	- GetStats()
	- WaitForPendingUpdate()
	- DefaultWaitForPendingUpdate()
- Removing of intermediates interfaces
- Rewriting of sample and Readme for the new usage
- Add tests for every method

**To do**
- Client
	- [x] Rewrite Client
	- [x] Add Test to Client
- Index 
	- [x] Rewrite Index
	- [x] Add Tests to Index
	- [x] Adding test for `WaitForPendingUpdate()` Issue [related](meilisearch/integration-guides#1)
	- [x] Rewrite Document
	- [x] Add Tests to Document
	- [x] Rewrite Search
	- [x] Add Tests Search
	- [x] Rewrite Settings
	- [x] Add Tests Settings

Co-authored-by: alallema <[email protected]>
@alallema
Copy link
Contributor Author

bors r-

@bors
Copy link
Contributor

bors bot commented Jun 22, 2021

Canceled.

@alallema alallema requested a review from curquiza June 22, 2021 08:26
@alallema
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 22, 2021

@bors bors bot merged commit 7a43625 into main Jun 22, 2021
@bors bors bot deleted the rewrite-client branch June 22, 2021 13:24
@alallema alallema added the bug Something isn't working label Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change The related changes are breaking for the users bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong accepted status code on /health (Health() method) Moving test to testify Change the usage Add test for WaitForPendingUpdate method

4 participants