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

Add a parameter to disable configuration file automatic creation #45

Merged
merged 4 commits into from
Jan 4, 2025

Conversation

superdarki
Copy link
Contributor

@superdarki superdarki commented Oct 9, 2024

Changes

Why

In very specific cases, configuration files should not be created by the daemon before the server first startup as they have not been created during the installation process.
This new flag enables egg creators to specify (default true) if the file should be created or not when the daemon tries to parse it.

What it add

{Server File Config}
(This is just an example on the Vanilla Minecraft egg, this is not where I think the flag would be useful)

How

When the daemon tries to open the file to parse it, changing the function to Open instead of Touch makes it return an error instead of creating the file. If the error is "IsNotExist" and "create_file" is false just return without logging an error.

Config parser file open function edition :
{AC38805D-4F4F-41D7-9B60-2084BE086CC4}
Add the flag to the ConfigurationFile struct :
{E02BC110-3581-4054-A64D-61F0EECFCCDB}
UnmarshalJSON function addition :
{67C5CD79-627D-4A2E-A03E-2724037B8D74}

@superdarki
Copy link
Contributor Author

I have a bug with my pelican install (docker related), I can't test if the IsNotExist error handling works.

@superdarki superdarki marked this pull request as ready for review October 10, 2024 12:28
@superdarki
Copy link
Contributor Author

Tested it and now it works, the error handling and the not creation of the file

@theremygit
Copy link

UP

@gOOvER
Copy link

gOOvER commented Oct 21, 2024

UP

??

@QuintenQVD0
Copy link
Contributor

This now conflicts with this: #34

and is there anyone realy needing this?

@superdarki
Copy link
Contributor Author

Hi, sorry haven't really been able to check github for a while.
Idk if anyone other than me really needs this but I'll still fix it when I find the time this week.
I think it's still a neat addition if someone needs it (like me) and doesn't want the hassle in creating weird egg configs when they could have used this.

@superdarki
Copy link
Contributor Author

I finally took the time to fix the conflicts

@parkervcp
Copy link
Contributor

@superdarki
If this is a change to the file parser there needs to be a change on the panel side to match that. Do you have a change submitted for that as well?

@superdarki
Copy link
Contributor Author

Hi @parkervcp,
No I did not submit any change to the panel as it works without it but it might be a bit janky in how I did it.
I'll do that then and send the link here when published.

@parkervcp
Copy link
Contributor

Hi @parkervcp, No I did not submit any change to the panel as it works without it but it might be a bit janky in how I did it. I'll do that then and send the link here when published.

You can cancel on that. I read it as an egg feature and not part of the file json. This doesn't need a panel side change.

@parkervcp parkervcp merged commit cb6f528 into pelican-dev:main Jan 4, 2025
6 checks passed
@superdarki
Copy link
Contributor Author

Ok, perfect then.
Thank you very much

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.

5 participants