Repository navigation
Update the README to match the code - #61
Merged
Merged
Conversation
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
|
bmfmancini
approved these changes
Oct 25, 2026
This file contains hidden or 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
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.



docs: describe the plugin as it behaves today
Description
Rewrites
README.mdso each statement matches the default branch as it is now, including the mergedget_request_var()change (#58). The old README was a stub: it did not say what to install, which versions are supported, how the import works, which realm to grant, what the plugin stores, or how to run the tests, and it carried a note about a Cancel button that no page of this version draws. OnlyREADME.mdchanges. No code,INFOormanifest.jsonchange, because nothing in them was wrong.Base:
origin/mainat 34138d8.Related issue
None.
Motivation and context
An administrator reading the README could not tell what the plugin needs or what it does to the database. A few facts that matter in practice were nowhere in the repository's documentation: one realm covers viewing, uploading and deleting; the address and name columns are narrow, so IPv6 clients and MySQL's
Id:suffix are stored wrongly; the page lifts PHP's limits for every request; uninstalling drops all imported data.How has this been tested?
Every claim was checked against the code or by running it. The environment was a throwaway Cacti 1.2.x tree (a local branch at 1.2.32) with the plugin from
origin/mainand MariaDB 11.8.9, PHP 8.4. The commands and results are in the table below. Two statements rest on code or the workflow only and are marked as such.INFO:8compat = 1.2.29; the CI workflow clonesCACTI: 1.2.x(.github/workflows/plugin-ci-workflow.yml, env). Installed and ran on 1.2.32 here; 1.2.29 itself was not runfunction display_version(): void(import_log.php:175),?string $table_mode(includes/functions.php); matrixphp: ['8.2', '8.3', '8.4']and the "Check PHP Syntax for Plugin" step. Only 8.4 was run here; 7.1 was not runincludes/database.php:38-45;SELECT table_name, engine FROM information_schema.tableson MariaDB 11.8.9 showed Aria for_details,_details_methods,_details_tablesand InnoDB for the restpath_php_binaryfor uploadsslowlog.php:171andincludes/functions.php:1238start the importer with it; the upload test in the throwaway tree worked with the setting presentINFO:9remote_collect:0,offline_view:0,offline_mgmt:0layout.json Cacti before 1.2.31grep cactiReturnTofinds one use in the plugin,slowlog_save_button()inslowlog.php, which no code calls. No page of this version draws such a button. (Core did gain the handler in 1.2.31:release/1.2.31:include/layout.js:775.) #60 adds No and Return buttons that use it, so its README change should restore the noteplugin_manage.php --install --enableplugin_configrowslowlog / 2.6 / status 1SHOW TABLES LIKE 'plugin_slowlog%'listed the eight tables of the README; 776 words inplugin_slowlog_reserved_words;includes/database.php:265readsdocs/keywords.txtadminplugin_realmsrow2 / slowlog / slowlog.php / Plugin -> MySQL Slow Log Viewer;user_auth_realmrow for user 1, realm 102;setup.php:59setup.php:59is the onlyapi_plugin_register_realm; the upload (form_save) and delete (form_actions) handlers are inslowlog.php, which that realm gates; an upload and the other pages all answered 200 for a user holding only that realmsetup.php:263slowlog_config_settings()adds an empty array to the Misc tabplugins.phporslowlog.phpwhen the version differs; prunes tombstones,tests/andphpunit.xml; warns about unlisted filesplugin_config.versionto 2.5 and ranslowlog_check_upgrade():themes/,keywords.txt,tests/andphpunit.xmlwere removed, the version went to 2.6, and the Cacti log showed "slowlog upgrade found a file/directory not described in manifest.json: stray.txt (left in place)";setup.php:362-472--disable: status 4, 8 tables left--disablethen--uninstall: 0plugin_configrows, 0 tables, 0 realm rows, 0 grants, 0 hooksimport_file,description,length,table_mode,table_names, and the "Max Upload Filesize / Max Post Size / Max Execution Time / Memory Limit" rows; length choices All, 1024 to 16384 Chars; the three table modes (HTML read back)--delete-afterin the worker commandslowlog.phpstatus labels;refreshMSeconds = 5000(slowlog.php:1884) when a row has status 0php plugins/slowlog/import_log.php --helpprinted the usage;--table-mode=xgives "Invalid --table-mode value";--table-mode=referencewithout names gives "requires --table-names"; options read from the parser (import_log.php). The help text is wrong for--usecacti(it says "The logid ..."); the README describes the behaviour, not that line--delete-afterdeletes also for a file that is not a logimport_log.php:149; ran an import of a non-log file with a pre-created record and--delete-after: the record went to status 3 (Bad File Format) and the file was gone afterwards--lengthcuts bytesincludes/functions.php:1002substr()--table-namesis given without--table-modeimport_post_process()inferslist; imported a log with--table-names='host graph_local'and no mode: onlygraph_local,hostwere associated, while the default mode foundgraph_local,host,settingsschemastored asshop, status 2Id:ends up in the addressId:suffix: addresses stored127.0.0.1 Idand192.168.100.100(cut at 15);includes/functions.php:1069splits the line on every colon::1stored as'',2001:db8::17stored as2001;ip_addressisvarchar(15)(includes/database.php:72)includes/database.php:70,76,181SET timestamp=and use PHP's time zonedate()of that epoch in every import above; the E2E failure of the MySQL 8 import underAmerica/New_York(reproduced locally) comes from the same source. Fixed in #60raw_count,raw_query,raw_examined,raw_sent,raw_affected,raw_bytes) and five box charts (box_querytobox_bytes); "Include Max" is inslowlog.php:1538slowlog.php:32-33locales/build_gettext.shls locales/po/*.po | wc -l= 21,locales/LC_MESSAGES/*.mo= 21; ran the script in a copy: it regenerated the template and all.po/.mofiles (needsxgettext,msgmerge,msgfmt,realpath; it also printed abasenameusage line on macOS and carried on)plugins/slowlogandtests/.cacti-versionset todevelop:include/vendor/bin/pest --configuration=plugins/slowlog/phpunit.xmlgave "271 passed, 1 skipped".composer.jsonof the 1.2.x tree used here has no Pest; develop's haspestphp/pest--coverage-clover=plugins/slowlog/coverage/clover.xmlrun passed;php tests/bin/patch-coverage.php <clover> origin/main 100ran in the git checkout and printed "no measured lines changed" (the README's relative pathcoverage/clover.xmlis the workflow's form)validate-manifest.php,check-i18n-pot.php origin/mainNot verified: Cacti 1.2.29 and 1.2.30 themselves; PHP 7.1 to 8.1; MySQL (as opposed to MariaDB) as the Cacti database; the SlowLog tab in the Cacti header (the hook is registered in
setup.php:56-57but I did not check its rendering, so the README does not describe it).Known issues I did not put in the README
The No button on the delete confirmation currently deletes the selected logs (fixed in #60), so the README only says to confirm with Yes. I did not document defects that an open PR fixes.
Types of changes
Checklist