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

implement webserver first commit #193

Merged
merged 23 commits into from
Oct 20, 2024
Merged

implement webserver first commit #193

merged 23 commits into from
Oct 20, 2024

Conversation

nishikawaakira
Copy link
Collaborator

@nishikawaakira nishikawaakira commented Oct 7, 2024

Web Server functionality has been implemented.
This allows dynamic searches in the GUI.
It also makes links to rules refer to the repository.

@nishikawaakira nishikawaakira marked this pull request as draft October 7, 2024 23:17
@nishikawaakira nishikawaakira marked this pull request as ready for review October 14, 2024 01:56
Copy link
Collaborator

@fukusuket fukusuket left a comment

Choose a reason for hiding this comment

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

@nishikawaakira
I got an error that common.js is missing, am I missing something in my procedure?🤔
(I copied the templates folder in the feature-webserver branch before executing the start-server command.)

fukusuke@fukusukenoMacBook-Air takajo-2.6.0-mac-arm % ls -la ./templates
total 1304
drwxr-xr-x@ 10 fukusuke  staff     320 10 14 15:51 .
drwx------@ 10 fukusuke  staff     320 10 14 15:49 ..
-rw-r--r--@  1 fukusuke  staff   44659 10 14 15:51 alpinejs.3.14.1.js
-rw-r--r--   1 fukusuke  staff    9391 10 14 15:51 common.js
-rw-r--r--@  1 fukusuke  staff   14552 10 14 15:51 computer_summary.template
-rw-r--r--@  1 fukusuke  staff   18011 10 14 15:51 content.template
-rw-r--r--@  1 fukusuke  staff   83961 10 14 15:51 font-awesome.6.0.css
-rw-r--r--@  1 fukusuke  staff   14943 10 14 15:51 index.template
-rw-r--r--@  1 fukusuke  staff  470394 10 14 15:51 tailwind.3.4.js
drwxr-xr-x@  4 fukusuke  staff     128  9 23 13:14 webfonts
fukusuke@fukusukenoMacBook-Air takajo-2.6.0-mac-arm % ./takajo start-server -s ./html-report.sqlite -p 8089
syncio.nim(767)          open
Error: unhandled exception: cannot open: ./src/takajopkg/web/static/js/common.js [IOError]
fukusuke@fukusukenoMacBook-Air takajo-2.6.0-mac-arm %

@fukusuket
Copy link
Collaborator

fukusuket commented Oct 14, 2024

I would also appreciate it if you could confirm the following two points🙏

  1. The . /takajo help command, the start-server command is not shown, so it would be better to show it!
  2. It would be good if we can delete unnecessary imports.(If the warning is wrong, no action is needed!)
fukusuke@fukusukenoMacBook-Air takajo % nimble -v                           
nimble v0.16.1 compiled at 2024-10-14 06:36:25
git hash: couldn't determine git hash
fukusuke@fukusukenoMacBook-Air takajo % nim -v
Nim Compiler Version 2.0.10 [MacOSX: amd64]
Compiled at 2024-10-14
Copyright (c) 2006-2023 by Andreas Rumpf

active boot switches: -d:release

fukusuke@fukusukenoMacBook-Air takajo % nimble build -d:release --threads:on
...
/Users/fukusuke/Scripts/Nim/takajo/src/takajopkg/web/controllers/controller.nim(1, 8) Warning: imported and not used: 'json' [UnusedImport]
/Users/fukusuke/Scripts/Nim/takajo/src/takajopkg/web/controllers/controller.nim(3, 8) Warning: imported and not used: 'computers' [UnusedImport]
/Users/fukusuke/Scripts/Nim/takajo/src/takajopkg/web/controllers/rules.nim(5, 8) Warning: imported and not used: 'times' [UnusedImport]
/Users/fukusuke/Scripts/Nim/takajo/src/takajopkg/web/startServer.nim(2, 28) Warning: imported and not used: 'staticfile' [UnusedImport]

@YamatoSecurity
Copy link
Collaborator

@nishikawaakira Sorry, can you rename start-server command to html-server ? It will go nicely with html-report since they are similar.

@YamatoSecurity
Copy link
Collaborator

@nishikawaakira 横からすみません。 I changed the name from start-server to html-server and removed some of the unused imports. I also added the examples and updated some wording. Please check my commits.

Questions:

  1. There is still one more unused import: takajo/src/takajopkg/web/controllers/controller.nim(2, 8) Warning: imported and not used: 'computers' [UnusedImport] There seems to be a computers.nim file with code in it, but is it not being used?
  2. The usage for this command is html-server -s ./takajo/sqlite.db -p 8089 but how do we first create the sqlite.db file? I think we should make the usage similar to html-report by using the timeline.jsonl as INPUT and have the command create the sqlite DB, what do you think? The other option is to create a new command html-create-db that creates the SQLITE database and then we can use that database as input to both html-report and html-server. Please tell me what you think the best option is. If we do the latter with the html-create-db command, then we only need to create the database once and then we can run both html-report and html-server later so it may increase performance.

P.S. @fukusuket I went ahead and changed the LICENSE to AGPL so we don't need to create another PR for it.

@nishikawaakira
Copy link
Collaborator Author

@YamatoSecurity @fukusuket
Sorry for the late response.

There is still one more unused import

It was unnecessary code, so I removed it.

The usage for this command is html-server -s ./takajo/sqlite.db -p 8089 but how do we first create the sqlite.db file?

The html-server command also creates a sqlite file, since we need a separate table from the html-report, I think it is better to create it with each command

Copy link
Collaborator

@fukusuket fukusuket left a comment

Choose a reason for hiding this comment

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

Thank you so much for implementation! LGTM!!🚀

Copy link
Collaborator

@YamatoSecurity YamatoSecurity left a comment

Choose a reason for hiding this comment

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

@nishikawaakira LGTM! Thanks so much!

@YamatoSecurity YamatoSecurity merged commit 3ce2d0c into main Oct 20, 2024
2 checks passed
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.

3 participants