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

refactor frontend with routing because we need more api routes f… #32

Merged
merged 21 commits into from
Nov 9, 2024

Conversation

vgarcia007
Copy link
Collaborator

@vgarcia007 vgarcia007 commented Sep 17, 2024

Problem:
The current frontend architecture is becoming difficult to manage, particularly as more API routes are needed for the file manipulation feature #27 . Without proper routing, the system will become increasingly unmanageable.

Solution:

  1. Implement Frontend Routing:
    Refactor the frontend to introduce routing mechanisms, which will help organize and streamline the handling of multiple API routes, especially for the file manipulation feature. This ensures scalability and clarity in the codebase.

  2. Server Configuration Update:
    Inject mod_rewrite into the web server configuration (Lighttpd) to handle clean URLs and route requests properly.

  3. File Structure Reorganization:
    Move sensitive files (functions, config, and classes) to a private directory outside of the document root. This prevents direct access via the browser.

  4. Timezone Issue Fix Wrong Timezone in php #26 :
    Since Lighttpd does not pass environment variables to PHP, the timezone must now be manually set in the configuration file. Update the config.php to include the correct timezone settings to ensure consistency across the application. (this is done via the startup shell script)

  5. Dockerfile Update:
    Modify the Dockerfile to accommodate the changes in the directory structure. The new directories (like the private directory) should be reflected in the container's build process to ensure everything is properly mapped and accessible inside the container.

Next Steps:

  • Add file rename and delete in gui and api routes
  • Add error responses for gui and api routes
  • Add swagger for api docs

…or the file manipulation feature.

 - inject mod rewrite to webserver condig
 - moved function, config and classes to private directory
 - new try for timezone issue. writing it to config because lighttpd dosent hand over env vars to php
 - Dockerfile needed change for new dirs
@gstrauss
Copy link

FYI: you can use lighttpd mod_setenv to set variables in lighttpd that are propagated to a FastCGI backend. I have not checked if PHP will honor something like TZ sent per-request, which is after the PHP has started up.

@PhilippMundhenk
Copy link
Owner

Ignore the "checks failed" for now. Seems the PRs are building now, but some permission issues when pushing images from forks to my gh packages. Will try to fix this in the coming days...

@vgarcia007
Copy link
Collaborator Author

FYI: you can use lighttpd mod_setenv to set variables in lighttpd that are propagated to a FastCGI backend. I have not checked if PHP will honor something like TZ sent per-request, which is after the PHP has started up.

Thanks for the tip. We should take a closer look at this for the future. Currently, I have a solution (writing to the config.php, like with the other variables).

@vgarcia007
Copy link
Collaborator Author

A quick update from my side. I won't be able to finish everything this week. I'll make sure the branch is ready next week.

@PhilippMundhenk
Copy link
Owner

Thank you for all the efforts! There is really no rush on this.

…a php. lets wait and integrate the new python files and check if we can change it back
@vgarcia007 vgarcia007 changed the title WIP: refactor frontend with routing because we need more api routes f… refactor frontend with routing because we need more api routes f… Oct 4, 2024
@vgarcia007 vgarcia007 marked this pull request as ready for review October 4, 2024 15:01
@PhilippMundhenk
Copy link
Owner

This looks really good to me! I left some comments, mostly minor or wishes. Not that I can judge any of the web things, will have to try it. Not sure why the build does not work. Shall we merge this into the new development branch, then the :development container should be available for live testing. I think this should also not clash (much) with the python rewrite in #48. Should be fairly easy integration, as the interface remain the same brother scripts. Only the status handling, we will likely need to find a common way. But this should also be easier with python. Can you check file existance or contents for status, rather than checking for running process? This would likely be easiest...

@vgarcia007 vgarcia007 changed the base branch from master to development October 6, 2024 20:41
@gstrauss
Copy link

gstrauss commented Oct 8, 2024

FYI: you can use lighttpd mod_setenv to set variables in lighttpd that are propagated to a FastCGI backend. I have not checked if PHP will honor something like TZ sent per-request, which is after the PHP has started up.

Also, if lighttpd.conf is configured to start the FastCGI backend, lighttpd can be configured to set TZ before starting the PHP and that will definitely work. See lighttpd.conf fastcgi.server settings "bin-environment" and "bin-copy-environment"

@vgarcia007
Copy link
Collaborator Author

This looks really good to me! I left some comments, mostly minor or wishes. Not that I can judge any of the web things, will have to try it. Not sure why the build does not work. Shall we merge this into the new development branch, then the :development container should be available for live testing. I think this should also not clash (much) with the python rewrite in #48. Should be fairly easy integration, as the interface remain the same brother scripts. Only the status handling, we will likely need to find a common way. But this should also be easier with python. Can you check file existance or contents for status, rather than checking for running process? This would likely be easiest...

Yes, let's merge it into the development branch, along with Pull Request #48. Then we'll try to make the necessary adjustments.

@PhilippMundhenk
Copy link
Owner

PhilippMundhenk commented Oct 11, 2024

@vgarcia007 : Since this is a PR directly from your repo, could you do the conflict resolution, please? Probably doesn't help much, if I start it now, as I can't push it to your branch. It should be fairly trivial

@vgarcia007
Copy link
Collaborator Author

The conflicts are resolved, but it's not ready to merge yet. I still need to update the PHP code that triggers the scripts. It now has to trigger the new Python scripts instead of the previous .sh scripts. Testing this will take some time...

@vgarcia007 vgarcia007 changed the title refactor frontend with routing because we need more api routes f… WIP: refactor frontend with routing because we need more api routes f… Oct 31, 2024
@vgarcia007
Copy link
Collaborator Author

vgarcia007 commented Nov 4, 2024

Unfortunately, there's been no progress here. Merging the development branch into this one broke almost everything I've done. The switch from Ubuntu to Python Slim as the base image hasn't made things any easier.

It's now ready to merge and is functioning as expected. The only issue is retrieving the 'Waiting Status' via the API, which was anticipated. Addressing this will require developing a new approach to export this status from the Python script.

Replaced bash calls in PHP scripts with Python.
@vgarcia007 vgarcia007 changed the title WIP: refactor frontend with routing because we need more api routes f… refactor frontend with routing because we need more api routes f… Nov 4, 2024
@PhilippMundhenk
Copy link
Owner

Excellent, thank you!! I will merge this in (still not sure why the build on this PR fails, but lets check if it works in branch). We can then use #60 to fix the status.

@PhilippMundhenk PhilippMundhenk merged commit 3d2b7c7 into PhilippMundhenk:development Nov 9, 2024
1 check failed
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.

Wrong Timezone in php
3 participants