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

fix(hosts) : cache non existing hosts (#27366) #29390

Merged
merged 11 commits into from
Aug 22, 2024
Merged

fix(hosts) : cache non existing hosts (#27366) #29390

merged 11 commits into from
Aug 22, 2024

Conversation

dsolistorres
Copy link
Contributor

@dsolistorres dsolistorres commented Jul 30, 2024 •
edited by github-actions bot
Loading

Proposed Changes

  • Non existing host added to host cache to improve performance

Checklist

  • Tests

This PR fixes: #27366

@dsolistorres dsolistorres self-assigned this Jul 30, 2024
@erickgonzalez erickgonzalez requested a review from wezell July 30, 2024 20:33
@dsolistorres dsolistorres linked an issue Jul 30, 2024 that may be closed by this pull request
Host lookups do not cache when there is no match for site key #27366
Closed
wezell
wezell reviewed Jul 31, 2024
View reviewed changes
Copy link
Contributor

@wezell wezell left a comment

Choose a reason for hiding this comment

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

Where does the 404Cache get cleared?

It should be cleared on every save of a host

wezell
wezell previously requested changes Aug 14, 2024
View reviewed changes
Copy link
Contributor

@wezell wezell left a comment

Choose a reason for hiding this comment

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

All the same issue, use UtilMethods.isSet(()->object.method())

@erickgonzalez erickgonzalez requested a review from wezell August 16, 2024 14:42
@dotcms-sonarqube dotCMS-sonarqube
Copy link

Quality Gate passed Quality Gate passed

Issues
13 New issues
3 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

jdotcms
jdotcms approved these changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@jdotcms jdotcms left a comment

Choose a reason for hiding this comment

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

I have already approved the changes in the past, however new changes were added and re-review were asked to do, again, changes looks good

@erickgonzalez erickgonzalez dismissed wezell’s stale review August 20, 2024 19:17

Will is too busy :) Sanchez and I double checked the PR and looks good, and tests are good

@erickgonzalez erickgonzalez added this pull request to the merge queue Aug 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 20, 2024
@erickgonzalez erickgonzalez added this pull request to the merge queue Aug 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 20, 2024
@erickgonzalez erickgonzalez added this pull request to the merge queue Aug 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 21, 2024
@erickgonzalez erickgonzalez added this pull request to the merge queue Aug 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 21, 2024
@erickgonzalez erickgonzalez added this pull request to the merge queue Aug 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Aug 21, 2024
@erickgonzalez erickgonzalez added this pull request to the merge queue Aug 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 21, 2024
@erickgonzalez erickgonzalez added this pull request to the merge queue Aug 22, 2024
Merged via the queue into master with commit 0b665e2 Aug 22, 2024
31 checks passed
@erickgonzalez erickgonzalez deleted the issue-27366-cache-hosts-not-found branch August 22, 2024 15:37
This was referenced Sep 25, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erickgonzalez erickgonzalez erickgonzalez approved these changes

@jdotcms jdotcms jdotcms approved these changes

@gortiz-dotcms gortiz-dotcms gortiz-dotcms approved these changes

@wezell wezell Awaiting requested review from wezell

Assignees

@dsolistorres dsolistorres

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Host lookups do not cache when there is no match for site key
5 participants
@dsolistorres @wezell @erickgonzalez @jdotcms @gortiz-dotcms