Integrate kw_bd to the project and add migration script
#836
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[UPDATE] |
davidbtadokoro
force-pushed
the
sql-db
branch
2 times, most recently
from
6a029b1
to
b960878
Compare
|
[UPDATE] |
|
[UPDATE] |
|
[UPDATE] |
davidbtadokoro
force-pushed
the
sql-db
branch
4 times, most recently
from
2f7edf1
to
87152db
Compare
|
[UPDATE] |
rodrigosiqueira
requested changes
Apr 20, 2023
|
[UPDATE] |
davidbtadokoro
force-pushed
the
sql-db
branch
6 times, most recently
from
1d01465
to
9c6daae
Compare
rodrigosiqueira
requested changes
Apr 26, 2023
rodrigosiqueira
requested changes
Apr 30, 2023
Prevent `format_values_db` from adding single quotes to NULL values. Signed-off-by: Rubens Gomes Neto <rubens.gomes.neto@usp.br> Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
davidbtadokoro
force-pushed
the
sql-db
branch
3 times, most recently
from
4d7f8bb
to
11fa315
Compare
davidbtadokoro
commented
May 11, 2023
Introduces the file that builds the model to be used by kw to store all data related to statistics options, pomodoro sessions and kernel config files. Adds a script to handle the data migration from the metadata files in `KW_DATA_DIR` to the newly introduced database. Current migrated data are: statistics and pomodoro reports, and kernel configs. The directories holding the metadata are renamed as such: - `KW_DATA_DIR`/statistics->`KW_DATA_DIR`/legacy_statistics - `KW_DATA_DIR`/pomodoro->`KW_DATA_DIR`/legacy_pomodoro - `KW_DATA_DIR`/configs/configs->`KW_DATA_DIR`/configs/legacy_configs - `KW_DATA_DIR`/configs/metadata->`KW_DATA_DIR`/configs/legacy_metadata Thanks: Magali Lemes <magalilemes00@gmail.com> for the help modelling the database. Signed-off-by: Rubens Gomes Neto <rubens.gomes.neto@alumni.usp.br> Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
…cture The kw kernel-config-manager test suite is outdated in terms of codestyle. Some ShellCheck violations like not double quoting to prevent globbing and word splitting, and more subtle violations like not using curly braces in string concatenations are present. Also, some refactorings can be made, like: - Encapsulate global variable declarations inside `oneTimeSetUp` (those were loose at the start of the file); - Encapsulate changing dirs in and out of the `SHUNIT_TMP_DIR` at the beginning and ending of most tests inside `setUp` and `tearDown`, respectively; - Fix the order of arguments in `assert_equals_helper` calls. The second to last argument should be <expected_value> and the last should be <actual_value>. This order was inverted in most of the file; - Use the `is_safe_path_to_remove` function every time an `rm` or `rm -rf` is used for safekeeping; - Fix variable named `ret`. It was used in most tests for the same purpose as the `output` variable used throughout the project (i.e. to capture the output of a command to assert something about it). Besides the name not following the project codestyle, it also was wrongly used, in some cases; - Change `assertTrue` calls by `assert_equals_helper` where convenient. The first function was used to assert if two values were exactly equal in some cases. For those, it is better to use the second function as it shows the expected and actual values with colors while the first just prints a message. This commit addresses the points described above to make the suite more maintainable and also adds some TODO comments where some further changes can be done that escape the scope of this commit. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
kw kernel-config-manager uses metadata files to manage the database management of kernel configuration files. This commit changes this by integrating kernel-config-manager with kw_db (that uses sqlite underneath). This is a more scalable approach and easier to maintain as some logic of data management is shifted to kw_db. The actual .config file is still stored in the user filesystem and just the metadata files were transferred to the database. Signed-off-by: Rubens Gomes Neto <rubens.gomes.neto@usp.br> Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
Much of the statistics is done via the `statistics_manager` function defined inside `kwlib.sh`. Those statistics are represented using metadata files. This commit integrates the function with kw_db and makes the necessary changes in `deploy.sh` and in `build.sh`, which are the callers. Signed-off-by: Rubens Gomes Neto <rubens.gomes.neto@usp.br> Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
In `kw deploy` feature, all commands tracked for statistics (except for full deploy) don't handle failure for statistics purposes. The failure of those commands is not handled at all. Either way, when a command fails, e.g. `kw deploy --list` ssh connection fails because the target remote machine is down, a status of `success` will be registered for this statistic in the database (in the example, `<ID>|list|success|<DATE>|<START_TIME>|<RUN_TIME>`). This commit makes `kw deploy` take account of the failure of all those commands when calling `statistics_manager`. We now capture the return value of the effective command handlers (run_kernel_install, modules_install, etc.) and use it to determine the `status` between `success` and `failure`. An interesting approach would be to pass the return value to `statistics_manager` and let it handle it. This would move the if and else statements from `deploy.sh` to `kwlib.sh` in the function definition. Also, it is worth pointing out that an `interrupted` command is not added to the statistics and we should consider handling this case in the future. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
The pomodoro.sh file doesn't follow the current project codestyle, like: - Having a standardized parser - Parser named 'parse_<feature-name>' - String concatenation without brackets - No parser error handling Also, the structure can be improved as some refactorings can be made, like method extractions, positioning of functions to reduce file scrolling and refactoring of options. This commit fixes those resulting in a more clean code. The actual size of the file grew, but this is due to the extraction of methods and their documentation. The 'kw pomodoro --tag' no longer shows all registered tags, instead this functionality is provided by the '--show-tags/-s' option. Also, the '--list/-l' option was renamed to '--check-time/-c' to be more indicative of its behavior. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
This commit fixes an unreachable command. This command was below a return statement, which rendered it unreachable. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
The same `timebox_to_sec` function is defined in the database migration script and in `report.sh`. Also, this function possibly will be used in `pomodoro.sh`. As such, this commit moves this function to the library file `kw_time_and_date.sh` where it can be included in the files listed above. The commit also adds documentation, error handling, and automated tests to the function. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
The pomodoro feature used metadata files both for data reports and for its functionalities (like showing registered tags). To take advantage of `kw_db` and to lessen the responsibilities of the pomodoro component (i.e. database management), this commit integrates `kw pomodoro` feature with `kw_db`. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
The `kw report` feature handles every type of report of kw tracked data. As all data used to be stored using metadata files, `report` is responsible for any logic related to fetching, processing, formatting, and outputting reports. There are two points worth noting: - The feature offers filters like by type of report (statistics or Pomodoro sessions) and periods of time (like a given day, week, month and year). The latter resulted in much of the feature's work being dealing with files hierarchy to fetch the proper data based on a period of time filter. - Also, the file doesn't follow a pattern. For example, the logic for implementing the period of time filters for the Pomodoro sessions report is defined in `report.sh` whilst the one for the statistics report is defined in `statistics.sh`. The responsibilities of each function are not clear, as some fetch, process, and format the data (statistics case) and others just fetch and partly process the data (Pomodoro sessions case). This commit solves the extensive logic implementation problem and lack of standardized code by integrating `kw report` with `kw_db`. A dedicated database and the power of SQL naturally solved the need for complicated logic for applying period of time filters to data fetching. This greatly reduced the complexity of the code and its size. The integration also allowed us to enforce a pattern in the file. This was done by: - Unifying the function to fetch the raw data (`set_raw_data`); - Adding a function to each type of report to process and format the data (`process_and_format_<statistics/pomodoro>_raw_data`); - Unifying the output function (`show_report`). Note: - This change resulted in a revision of the `report` test suite that greatly decreased its running time. - Just some processing functions like getting the maximum/minimum value from `statistics.sh` are still used. We could move them to `kwlib.sh` and deprecate `statistics.sh`. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
As per the project structure, the `kw_db` is a library file and should be placed in the `src/lib` directory. This commit does this by moving `kw_db` from the `src` directory to the `src/lib` directory and making the necessary adaptations throughout the project code. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
Active timeboxes for the feature `kw pomodoro --check-timer` have a time limit of 86399 seconds (23:59:59 in H:M:S format) for both the elapsed time and the remaining time. Because the function to convert time in seconds representation to format H:M:S representation assumed a 24-Hour Time Format (i.e. 00:00:00~23:59:59) a value of 72 hours (259200 seconds) will be displayed as 00:00:00. This commit fixes this by using the function `secs_to_arbitrarily_long_hours_min_secs` instead. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
When restoring a backup of kw data, if not using the `--force` flag, the statistics and pomodoro reports, and the kernel configs data are restored. This data is almost all the personal user data of kw (besides the kw config files). With the addition of `kw_db`, the file `KW_DATA_DIR/kw.db` needs to be restored as well. Also, the legacy metadata files and directories don't have the need to be restored and (in theory) should already be migrated into `kw_db`, leaving just `KW_DATA_DIR/kw.db` and the actual kernel config files to be restored. This commit does this by not calling the functions that used to restore the data mentioned in the first paragraph and adding and calling functions that restore the `kw_db` file and actual kernel config files. Note: As said before, this commit only affects the restoration without `--force` as, in this case, the full decompressed backup is just copied to `KW_DATA_DIR`. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
|
I merged this change in the unstable branch. Thanks a lot @kwy95 @davidbtadokoro @magalilemes for all the hard work. This is a fantastic change that put kw at another level :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a continuation of the work made by @kwy95 and @magalilemes in src: db: introduce db model and migration script (thanks for the excellent work!). Its purpose is to integrate
kw_bd, the implementation of a real database management system that uses SQLite underneath, into the project. In other words, wherever metadata files are being used to simulate aDBor whereverDB's functionalities are advantageous, this integration should happen. Also, this PR adds a script to migrate old metadata files into theDB.[TODOs] At the moment this PR is just a draft as there are several missing parts of it:
Updatekernel-config-managertest suite (integration is done)FInish integration withbuildanddeploy(statistics)Integrate withpomodoroIntegrate withreportFix migration script (migrate the same legacy user data only once)[TESTING THIS PR]
If you wish to test this PR (thanks a lot!), below are the steps and the assertions you should make. The TLDR is that just by pulling this PR, installing the PR version of kw and asserting the migration is consistent using
kw reportandkw kernel-config-manageris of great help! The steps below are a detailed script to fully test the PR (besides automated tests).kw backup <path_to_backup>orcp -r <kw_data_dir_path> <path_to_backup>unstablebranch updated withgit pull origin unstableunstablebranch withgit checkout unstablegit pull origin pull/836/head:<name_of_branch>git switch <name_of_branch>./setup.sh --install. You should see a warning likeCreating database: <kw_data_dir_path>/kw.dband no errors<kw_data_dir_path>/kw.dbhas been created<kw_data_dir_path>, the directoriespomodoroandstatisticsare renamed tolegacy_pomodoroandlegacy_statistics, respectively. For the kernel configs, assert that in your<kw_data_dir_path>/configs, the directoriesconfigsandmetadataare renamed tolegacy_configsandlegacy_metadataand that the actual kernel config files (the kernel.configs) are in<kw_data_dir_path>/configskw report <period of time>of a period of time that you can assert the outputkw build,kw deploy,kw deploy --list,kw deploy --uninstallorkw deploy --modules-deployand assert throughkw report --statisticstheir additionkw pomodoro --set-timer <timebox>and assert throughkw report --pomodorotheir addition. Also usekw pomodoro --check-timerto see if the active timeboxes are displayed correctlykw kernel-config-managerand assert that the feature works as intended./setup.sh --install. You should NOT see a warning likeCreating database: <kw_data_dir_path>/kw.db(also, no errors should happen)<kw_data_dir_path>hasn't changed and that there is no duplicated data. You could usesqlite3 <kw_data_dir_path>/kw.dband check that the tables are consistent (e.g.SELECT * FROM statistics_report;) or juts usekw report<kw_data_dir_path>and runningkw backup --restore <path_to_backup> --force)