Remove unused attributes and flags parameters from CreateChronicle and AcquireStory#641
Merged
Conversation
…and AcquireStory The attribute map argument was advertised by the public client API but never honored — ChronicleMetaDirectory only logged it and dropped it on the floor, no Keeper/Grapher/Store code consumed any value. Removing the parameter end-to-end (public client header, facade, impl, RPC, Visor portal, metadata directory, Chronicle::addStory) makes the API stop pretending to be configurable. Also removes the orphan tiering_policy field from ChronicleAttrs and StoryAttrs (always set to *_normal in the constructor, never read), the matching enums, and the "TieringPolicy=Hot" / "Priority=High" / etc. attribute-map setup in CLI, perf bench, examples, and integration tests. Closes #575.
e5eb674 to
46a742d
Compare
Resolve repo restructure (#563) conflicts: relocate ChronologClient.cpp modifications to client/cpp/src/.
The int& flags argument has been threaded through the public client API, the Visor portal, the Thallium RPC, and the metadata directory for years without ever being read or written — only logged. Removing it everywhere shrinks the API to what the system actually does and unblocks Python binding work that had to mirror the noise. Also drops a few orphan helpers exposed by the prior #575 cleanup: - Chronicle::removeArchive: no callers anywhere in the codebase. - Story::setProperty: only caller (Chronicle::addStory) was removed along with the attributes parameter; the function itself was left behind. - A pair of dead "Priority=High" string locals in client_metadata_rpc_test.cpp that were leftovers from the removed attribute-map setup. Closes #577.
fkengun
approved these changes
May 22, 2026
ibrodkin
approved these changes
May 22, 2026
…anges PR #641 dropped the attrs/flags parameters from Client::CreateChronicle and Client::AcquireStory. The chrono-pubsub adapter, merged in from develop, was still calling the old four-arg signatures and breaking the build. Update the calls to the new minimal signatures and drop the now- unused DEFAULT_FLAGS / story_attrs / chronicle_attrs locals.
This was referenced May 23, 2026
Closed
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.
The
CreateChronicleandAcquireStoryAPIs each took two parameters that have never done anything: anattributesmap and anint& flags. The attribute map was stored as opaque key/value strings and silently dropped; nothing in Keeper, Grapher, or Store ever consumed it. The flags reference was threaded through the public client, the Visor portal, the Thallium RPC, and the metadata directory for years, only ever to be logged. This PR removes both, end-to-end, so the v3.0.0 client API stops advertising configurability it doesn't deliver.These two changes were originally split into separate issues (#575 for attributes, #577 for flags) and separate PRs. Since they touch the same call sites in every example, test, and plugin, the second one to merge would have spent its time re-editing exactly the lines the first one touched. Bundling them into a single PR is cheaper and more honest about the fact that it's one logical change: "remove the parameters the API was lying about."
While in the area, this PR also clears out a handful of helpers that became orphaned by the cleanup:
Chronicle::removeArchivehas no callers anywhere in the codebase, so it goes.Story::setPropertywas only ever called fromChronicle::addStory, which lost itsattrsargument here; the function was left behind in the original attrs-removal slice and is removed now.Priority=Highstring locals inclient_metadata_rpc_test.cppthat the previous cleanup missed.tiering_policyfield and the matchingStoryTieringPolicy/ChronicleTieringPolicyenums — always set to*_normalin the constructor, never read.A few things are deliberately out of scope so this PR stays focused:
GetChronicleAttrandEditChronicleAttr, along with the underlyingpropertyList_/get_chronicle_attr/edit_chronicle_attrplumbing, still exist after this PR. They're dead in the same sense as the parameters we just removed, but they're removed as part of the v3.0.0 API surface freeze (Final C++ client API cleanup for v3.0.0 #660).*_action_is_authorized,is_client_authenticated,CL_ERR_NOT_AUTHORIZED) are still there; they're tracked under Remove authorization stubs from ChronoVisor #659.Local*stub block inVisorClientPortal.cppis also untouched — it gets deleted alongside the Python and CLI binding work in Expose chronicle metadata APIs in Python and CLI #573.Two notes for reviewers:
The Python binding file (
py_chronolog_client.cpp) didn't need any changes. pybind11 binds via&Client::CreateChronicleand&Client::AcquireStorymethod pointers, so the new C++ signatures are picked up automatically. The Python example scripts underclient/python/examples/are updated.In
plugins/chrono-stream/ClientScripts/client_writer.cpp, theacquire_or_create_storyfunction previously made three retry attempts that distinguished open-vs-create via differentflagsvalues. Since flags was never read on the server side, all three calls were always equivalent. The three-attempt structure is kept for now to minimize behavior change; the function can be collapsed in a follow-up if desired.Closes #575 and #577. Part of #572.