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

Improve FolderObserver #4444

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve FolderObserver #4444

wants to merge 1 commit into from

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Nov 10, 2024

Party fixes #3823

It has been reported that missing model directories cause issued during startup. With this PR a directory is created when it is missing.

It has been reported that missing model directories cause issued during startup. With this PR a directory is created when it is missing.

Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K J-N-K requested a review from a team as a code owner November 10, 2024 12:40
@lolodomo
Copy link
Contributor

Looks good

@J-N-K
Copy link
Member Author

J-N-K commented Nov 10, 2024

No, Not complete

@lolodomo
Copy link
Contributor

lolodomo commented Dec 7, 2024

Is it something we can include in 4.3 or is it still not yet ready?

@J-N-K
Copy link
Member Author

J-N-K commented Dec 7, 2024

Unfortunately I can't remember what is missing. IMO this is an improvement, even if it does not fully solve the issue. IIRC there are some corner-cases where it still does not work.

@holgerfriedrich
Copy link
Member

@J-N-K I remember that I had seen the discussion in #3823 if this should be solved by creating the missing folders or learn more about the root cause. This is why I did not merge it at that time.

So shall we proceed merging this one to improve the situation?
If yes, maybe we should add a log message in case the directory is created, just as hint that something unexpected happened.
WDYT?

@kaikreuzer
Copy link
Member

I have a general question here: The FolderObserver watches folders in the conf folder, right?
This folder is to be treated as read-only by openHAB (in contrast to the userdata folder), so creating directories in here does not look like the right approach to me.

@lolodomo
Copy link
Contributor

lolodomo commented Dec 8, 2024

Very good remark.
Maybe we should just log a warning to inform users that the folder is missing.
PS: it concerns only users using docker.

Comment on lines +100 to +105
try {
Path extensionPath = watchService.getWatchPath().resolve(extension);
Files.createDirectories(extensionPath);
} catch (IOException e) {
logger.error("Model path for extension '{}' is missing and creation failed: {}", extension, e.getMessage());
}
Copy link
Member Author

Choose a reason for hiding this comment

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

So the suggestion is

Path extensionPath = watchService.getWatchPath().resolve(extension);
if (!Files.isDirectory(extensionPath)) {
  logger.error("Model path for extension '{}' is missing or not accessible. Please create it and restart openHAB.", extension);
}

@wborn
Copy link
Member

wborn commented Dec 8, 2024

Maybe we should just log a warning to inform users that the folder is missing.
PS: it concerns only users using docker.

Yes some users just copy a few of their directories and think everything else will then magically work.
It's an issue for any distro so perhaps we should add something to the OH startup shell/batch files to detect this.

@Nadahar
Copy link

Nadahar commented Dec 8, 2024

I don't fully understand what causes this problem, but as one user explains in #3823:

So, for openhab to launch correctly, the openhab/conf/items folder must be present. If it is not there, then the problems will be described above. My system does not have this folder, items are stored in the file openhab/userdata/jsondb/org.openhab.core.items.Item.json. After an empty folder has been created. All things changed the status from NOT_YET_READY to Online the next time openhab was launched.

If no Items files are in use, why must this folder exist? It doesn't seem logical to me, whether it's created or logged, it seems like the real problem is why the lack of this folder causes the system initialization to fail.

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.

Thing manager logic causes things to stuck in NOT_YET_READY
6 participants