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

Fix missing index_uid in clean up job (#352) #353

Merged
merged 3 commits into from
May 7, 2024

Conversation

ellnix
Copy link
Collaborator

@ellnix ellnix commented Apr 20, 2024

Pull Request

Related issue

Fixes #352

curquiza
curquiza previously approved these changes Apr 22, 2024
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

thanks @ellnix

bors merge

meili-bors bot added a commit that referenced this pull request Apr 22, 2024
353: Fix missing index_uid in clean up job (#352) r=curquiza a=ellnix

# Pull Request

## Related issue
Fixes #352


Co-authored-by: ellnix <103502144+ellnix@users.noreply.github.com>
Copy link
Contributor

meili-bors bot commented Apr 22, 2024

Build failed:

@curquiza
Copy link
Member

I hate codecoverage, there is always something failing... 😅

@drale2k
Copy link

drale2k commented Apr 28, 2024

Hi, will this be merged?

@ellnix
Copy link
Collaborator Author

ellnix commented Apr 28, 2024

Hi, will this be merged?

I'm not familiar with Codecov (and don't have time to get familiar recently 🥲).

We have to wait for someone from Meilisearch staff who has the authority to merge this.

@curquiza
Copy link
Member

Invoking @brunoocasali here 🙏

@brunoocasali
Copy link
Member

I've regenerate the CODECOV_TOKEN let's see if that helps 😢

@curquiza
Copy link
Member

bors merge

meili-bors bot added a commit that referenced this pull request Apr 29, 2024
353: Fix missing index_uid in clean up job (#352) r=curquiza a=ellnix

# Pull Request

## Related issue
Fixes #352


Co-authored-by: ellnix <103502144+ellnix@users.noreply.github.com>
Copy link
Contributor

meili-bors bot commented Apr 29, 2024

Build failed:

@curquiza
Copy link
Member

@brunoocasali it still does not work 😢

@brunoocasali
Copy link
Member

Hi @ellnix can you take a look at the failing tests? Maybe the codecov issue will be gone as soon as we fix the tests.

@ellnix
Copy link
Collaborator Author

ellnix commented Apr 30, 2024

Hi @ellnix can you take a look at the failing tests? Maybe the codecov issue will be gone as soon as we fix the tests.

@brunoocasali test regressions fixed, codecov is still bugging.

@brunoocasali brunoocasali force-pushed the fix-async-delete-regression branch from c43e233 to 8e8b5f8 Compare May 7, 2024 17:16
@brunoocasali
Copy link
Member

Look what I found in the docs https://github.com/codecov/codecov-action?tab=readme-ov-file#usage

image

So, I replaced it to use the arg instead of the env, didn't work. Then I added both, didn't work, then I tried to create my own branch without the fork didn't work either.

But, I ran out of time to fix it, so I made a commit removing the codecov.

@brunoocasali
Copy link
Member

Guess what? Even after removing the GithubAction, we still have the issue because we have an outdated version of the codec gem installed (I didn't remember we were using one).

@brunoocasali brunoocasali force-pushed the fix-async-delete-regression branch 3 times, most recently from 68ca74a to 7c2465e Compare May 7, 2024 17:34
Copy link

codecov bot commented May 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.22%. Comparing base (9f71936) to head (7c2465e).

❗ Current head 7c2465e differs from pull request most recent head 30d5ed0. Consider uploading reports for the commit 30d5ed0 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #353   +/-   ##
=======================================
  Coverage   90.22%   90.22%           
=======================================
  Files          13       13           
  Lines         757      757           
=======================================
  Hits          683      683           
  Misses         74       74           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brunoocasali brunoocasali force-pushed the fix-async-delete-regression branch from 7c2465e to 30d5ed0 Compare May 7, 2024 17:39
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

bors merge

@brunoocasali brunoocasali merged commit 22795de into meilisearch:main May 7, 2024
9 of 10 checks passed
@ellnix ellnix added the bug Something isn't working label May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MeiliSearch::Rails::MSCleanUpJob 404 error when deleting records
4 participants