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.

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

Add missing fields: TextMatchInfo, HnswParams and CreatedAt. Also fixes failing tests because the Store field. #237

Merged
merged 6 commits into from
Aug 24, 2024

Conversation

solvingproblemswithtechnology
Copy link
Contributor

Hey there!

I made some time to send a PR with some missing fields!

Thank you!

@solvingproblemswithtechnology
Copy link
Contributor Author

Not sure why the tests are failing at CircleCI, for me everything passes and the given errors were fixed already.

@runeanielsen
Copy link
Member

Hi @solvingproblemswithtechnology, I'll take a look at why the tests are failing in the next couple of days.

// Rune

@solvingproblemswithtechnology
Copy link
Contributor Author

It seems like there's more collections that the ones created by the tests themselves. I guess you need to reset that typesense instance so it's clean again.

@runeanielsen
Copy link
Member

runeanielsen commented Aug 23, 2024

The instance is always clean on each PR, so it has to be something with the code you have added, since everything works before the change, but I'll look into it in a few days.

@solvingproblemswithtechnology
Copy link
Contributor Author

Nope, the tests were failing before I changed anything, it had something to do with the field Store being null instead of true (the default value when creating a collection). But yes, I'll wait for your input.

@runeanielsen
Copy link
Member

Hi @solvingproblemswithtechnology,

I've tried running the tests locally on the master branch HEAD commit and all tests pass without any issue, I've also repeated it multiple times without any issues. After that I tried checking out your PR and the tests fails, just like in the CI.

If you have problem with running the tests locally on the current HEAD of the master branch, lets try to fix that first before looking into this PR.

What OS are you running and are you sure that your Typesense test instance is 100% clean?

@solvingproblemswithtechnology
Copy link
Contributor Author

Windows and yes!

I'll try again and update the PR, thanks for testing it!

@solvingproblemswithtechnology
Copy link
Contributor Author

Just checked, the problem is that I was using the latest version, 27.0.rc35. With that version, it works. Using the 26.0, tests doesn't passes. To be clear, I didn't add those fields, and it will need a later fix when 27 is released.

image

image

@runeanielsen
Copy link
Member

runeanielsen commented Aug 24, 2024

Cheers @solvingproblemswithtechnology,

I'll keep that in mind when I upgrade the library to target 27.0.

@runeanielsen runeanielsen merged commit efafef4 into DAXGRID:master Aug 24, 2024
2 checks passed
@runeanielsen
Copy link
Member

@solvingproblemswithtechnology thanks for the contribution, I've made a new release (7.14.0) with the changes. 👍

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