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

docs(algolia): use one search-index per branch #2513

Merged
merged 11 commits into from
Dec 30, 2023

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Oct 28, 2023

Tries to fix: #2267


I couldn't find a way to actually test the changes.

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 2-high Fix main branch labels Oct 28, 2023
@ST-DDT ST-DDT added this to the v8.x milestone Oct 28, 2023
@ST-DDT ST-DDT requested review from a team October 28, 2023 20:57
@ST-DDT ST-DDT self-assigned this Oct 28, 2023
@codecov
Copy link

codecov bot commented Oct 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a87602b) 99.57% compared to head (9d97c66) 99.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2513      +/-   ##
==========================================
- Coverage   99.57%   99.57%   -0.01%     
==========================================
  Files        2807     2807              
  Lines      250379   250379              
  Branches     1143     1139       -4     
==========================================
- Hits       249322   249307      -15     
- Misses       1029     1044      +15     
  Partials       28       28              

see 1 file with indirect coverage changes

@Shinigami92
Copy link
Member

This looks straight forward to me 🤔
Should we test in production?

Shinigami92
Shinigami92 previously approved these changes Dec 28, 2023
@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 28, 2023

I kind of already did, but the index doesn't get populated, so we would loose the current search capabilities.

@Shinigami92
Copy link
Member

I kind of already did, but the index doesn't get populated, so we would loose the current search capabilities.

That would not be a problem for me
The index is new, so the data is also reset
It will be build up again

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 28, 2023

It will be build up again

No, it won't. At least not for the last 5 months...

@xDivisionByZerox
Copy link
Member

It will be build up again

No, it won't. At least not for the last 5 months...

We would also need to backport this PR to v6 to prevent potential conflicts with the newly created indexes. There souldn't be any, but better be safe I guess.

@Shinigami92
Copy link
Member

It will be build up again

No, it won't. At least not for the last 5 months...

we could talk about what you tested and how maybe later in a voice-call or team-meeting
I'm a bit confused, or at least don't understand what an index is then

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 28, 2023

grafik

I'm a bit confused, or at least don't understand what an index is then

There is nothing to be confused about.
(It normally gets updated during CI build)
The index not getting updated, is a bug or misconfiguration.
Maybe merging #2301 might fix it or it might be entirely unrelated to that.

@Shinigami92
Copy link
Member

do we maybe need to create an index there? with that button?
and then it gets filled and connected with the one in the code?

the one fakerjs index that is there already has 6.13K records, so as far as I understand the index dynamically builds up, based on what people are searching on the docs

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 28, 2023

Yes, you might have to create a new index, but even then it stays empty after the website build/time.
I can show it to you tomorrow, if you want.

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 29, 2023

The crawler is hosted and run on https://crawler.algolia.com/.
There appears to be some kind of failsafe, that prevents major changes to the index without human intervention.
For some reason this triggered 5 months ago and thus the index hasn't been updated since. (Fixed now)

We checked the configuration and noticed that we have to add the other subdomains to the whitelist.
This is currently in progress, once we did that, we can merge this and have individual indexes.
But we have to check periodically whether they still work. 😿

Thanks @Shinigami92 for the help debugging this.

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 29, 2023

FFR: The index-names have to start with fakerjs 🤷‍♂️

grafik

docs/.vitepress/versions.ts Outdated Show resolved Hide resolved
@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 29, 2023

This now works as expected:

Ref:

@ST-DDT ST-DDT marked this pull request as ready for review December 29, 2023 17:00
@ST-DDT ST-DDT requested review from a team December 29, 2023 17:00
@ST-DDT ST-DDT merged commit c10899b into next Dec 30, 2023
20 checks passed
@ST-DDT ST-DDT deleted the docs/algolia/index-per-branch branch December 30, 2023 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation p: 2-high Fix main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Searching unique on website suggests page that does not exist
4 participants