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

Fix installer migration #6

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

jameskitt616
Copy link

  • I added a new generateKey form to public/install/forms.php because the env variable apparently isnt being loaded properly if the key generation and database migration is in one form. And the DB migration needs the APP_KEY loaded
  • I formatted a few Lines in public/install/index.php. But the only new stuff happened from Line 233.

@S0ly
Copy link
Owner

S0ly commented Jul 1, 2024

@jameskitt616

@jameskitt616
Copy link
Author

@S0ly why did you ping me without a message?

S0ly
S0ly previously requested changes Jul 2, 2024

// DB Migration & APP_KEY Generation
Copy link
Owner

Choose a reason for hiding this comment

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

I dont get why instread of fixing the bug you created a totally new form that is not really needed ?

@S0ly
Copy link
Owner

S0ly commented Jul 2, 2024

I don't know why my GitHub is bugged

@S0ly S0ly self-requested a review July 2, 2024 09:56
@S0ly
Copy link
Owner

S0ly commented Jul 2, 2024

I was saying I dont get why instread of fixing the bug you created a totally new form that is not really needed ?

@jameskitt616
Copy link
Author

I tried "fixing the bug" but apparently when setting the env variables AND trying to access it instantly (in the same form for the migration) it doesnt really work. probably because the state is not updated yet.
I tried to fix it with updating the env variables manually and clearing the settings cache via laravel command during runtime, but it didnt work.
If you got a better idea how to fix it, or know laravel better than i do, i am glad to have some help.
(i also don't think it's to bad to have an extra form for this, but i do agree that it's kinda weird to have)

@S0ly
Copy link
Owner

S0ly commented Jul 2, 2024

have you tried to add a sleep for like 3 seconds ?

@jameskitt616
Copy link
Author

I tried to add a sleep for 1s but that should be plenty of time to set a env variable

@jameskitt616
Copy link
Author

So what is happening with this now?

@S0ly
Copy link
Owner

S0ly commented Jul 9, 2024

you didnt told me if adding a sleep fixed the problem ? is there a more simple less overkill solution than creating a new form ?

@jameskitt616
Copy link
Author

I already told you at least twice that a sleep of 1s didnt solve it. And 1s should be plenty of time for this.

is there a more simple less overkill solution than creating a new form ?

yes, creating the APP_KEY in the previous step should do the trick as well.
Should i do that?

@S0ly
Copy link
Owner

S0ly commented Jul 9, 2024

since checkDB step set all the env value related to DB maybe yes it could be great to merge it whit this step :)

@jameskitt616
Copy link
Author

Alright will do, maybe i find the time today. I will let you know here when i am done.

@jameskitt616
Copy link
Author

Updated it so it will generate the APP_KEY now in the database setup. Please review the MR.

@S0ly
Copy link
Owner

S0ly commented Jul 9, 2024

you tested it and it work ?

@S0ly
Copy link
Owner

S0ly commented Jul 9, 2024

my GitHub have the review process that is glitched I dont know why :(

@jameskitt616
Copy link
Author

Yes tested it for normal case and for a exception case. both worked.

@S0ly
Copy link
Owner

S0ly commented Jul 9, 2024

okay :) thanks you

@S0ly S0ly merged commit 9b989a5 into S0ly:fix/panel-installer Jul 9, 2024
@jameskitt616 jameskitt616 deleted the fix-installer-migration branch July 9, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants