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

Improving "internal reload" during project reload #7748

Merged
merged 2 commits into from
Sep 18, 2024

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Sep 12, 2024

Seems awkward... but sometimes events are delivered in the middle of a project loading process. The API checks the project's state at the end for quality and/or consistency - and that can cause spurious errors about project reloads while reading it. This PR improves stability by allowing a recovery - but at the same time if a reload would be repeated again because of file changes, the ProjectReload.withProjectState() fails indicating that project is being modified, as it was before. Similar approach was already implemented for the case the ProjectReloadImplementation requested a retry: one is OK, but the same (super)set repeated again will result in an error.

During investigation, I needed quite detailed logging, so I've added that as a separate commit; the "logging" commit should bring no functional changes, just diagnostics.

@sdedic sdedic added Gradle [ci] enable "build tools" tests LSP [ci] enable Language Server Protocol tests Maven [ci] enable "build tools" tests Platform [ci] enable platform tests (platform/*) labels Sep 12, 2024
@sdedic sdedic added this to the NB24 milestone Sep 12, 2024
@sdedic sdedic force-pushed the sdedic/project-internal-reload branch from 42dac65 to 664269f Compare September 12, 2024 10:30
@sdedic sdedic self-assigned this Sep 12, 2024
@sdedic sdedic requested a review from lahodaj September 12, 2024 14:13
@sdedic sdedic assigned jhorvath, dbalek and sdedic and unassigned jhorvath, dbalek and sdedic Sep 12, 2024
Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

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

No objections from me.

@sdedic sdedic force-pushed the sdedic/project-internal-reload branch from 664269f to 2d925f2 Compare September 14, 2024 13:32
@sdedic
Copy link
Member Author

sdedic commented Sep 14, 2024

Rebased on latest master; @mbien review comments addressed.

@mbien
Copy link
Member

mbien commented Sep 15, 2024

Rebased on latest master; @mbien review comments addressed.

ok thanks, don't forget to squash the third commit into the other commits before merge though.

@sdedic
Copy link
Member Author

sdedic commented Sep 15, 2024

ok thanks, don't forget to squash the third commit into the other commits before merge though.

Sure, waiting until Mon for other possible review comments + fixes.

@sdedic sdedic force-pushed the sdedic/project-internal-reload branch from 2d925f2 to 9db7581 Compare September 18, 2024 15:24
@sdedic
Copy link
Member Author

sdedic commented Sep 18, 2024

Fixes merged into 2nd commit; prepare for merge.

@sdedic sdedic merged commit 0374e1f into apache:master Sep 18, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gradle [ci] enable "build tools" tests LSP [ci] enable Language Server Protocol tests Maven [ci] enable "build tools" tests Platform [ci] enable platform tests (platform/*)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants