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

InitConfigDir is not platform agnostic #1834

Open
turetske opened this issue Dec 17, 2024 · 2 comments
Open

InitConfigDir is not platform agnostic #1834

turetske opened this issue Dec 17, 2024 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@turetske
Copy link
Collaborator

In the case where the $HOME or configDir directory isn't set, InitConfigDir makes an assumption that it's running on Linux.

This is rare case that could occur on Windows, but hasn't yet, so is probably why it hasn't been caught.

The InitConfigDir should be made platform agnostic and the tests should be allowed to run on windows to ensure everything works. We may need a discussion on what fall back config directory should be when not on Linux (as the code uses "/etc/pelican", which is a Linux path).

Basically, we want to ensure that for pelican clients running on either windows or linux, if the ConfigDir isn't set and the user isn't root, we have a fallback location that the client attempts to use on both.

@turetske turetske added the bug Something isn't working label Dec 17, 2024
@bbockelm
Copy link
Collaborator

@turetske - can you drop links into where the code makes these assumptions?

In general, the CI tests are indeed run on Windows -- are there specific ones that are currently marked as Linux-only that should be extended to Windows as well? If so, can you drop links so one can easily pick up the ticket?

@turetske
Copy link
Collaborator Author

turetske commented Dec 17, 2024

@bbockelm Ah.

So we didn't have CI tests testing this function with this specific case (What happens if "ConfigDir" isn't set)

I found this problem when writing these specific tests which then failed on windows.

The location this occurs is here

This code in this function was originally part of InitServer so the linux specifics were fine, then. When it became its own function that was used by the general InitConfig is when this was introduced, because this wasn't changed.

This ticket should be complete with #1836 which identifies the larger problem with how we structured this code.

@turetske turetske self-assigned this Dec 17, 2024
@turetske turetske added this to the v7.14 milestone Dec 17, 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

No branches or pull requests

2 participants