Skip to content

Add cacheConfig, setter process, getter script#743

Open
bsofjan wants to merge 2 commits into
DataIntellectTech:masterfrom
bsofjan:configsettergetter
Open

Add cacheConfig, setter process, getter script#743
bsofjan wants to merge 2 commits into
DataIntellectTech:masterfrom
bsofjan:configsettergetter

Conversation

@bsofjan

@bsofjan bsofjan commented May 29, 2026

Copy link
Copy Markdown

Per Phase 1.3 Setter Process and Phase 1.4 Getter Bootstrap of the Anycache Project Proposal, in this PR I am adding:

  1. Setter process
  2. Getter bootstrap
  3. cacheconfig json

The goal of Phase 1.3 and Phase 1.4 is to put together the bare bones of a "setter" process and "getter" functionality.

@CJA18

CJA18 commented Jun 2, 2026

Copy link
Copy Markdown

A few general things you need to do when committing to TorQ:

  • The PR needs a more detailed description outlining the reason for it, what's being achieved, future steps to come etc
  • A lot of these functions seem to be something common that has likely been done already and might have an existing util function, always check for this first as it's best practice to reuse the existing utils (if something needs to be changed then it only needs to be changed in one place)
  • Follow coding standards, if you look at the rest of the codebase camelCase isn't used so follow whatever standard is used in the codebase
  • For comments, in the rest of the codebase you'll see that the comments are generally block comments for each function describing what the function and each step in the function is doing, and inline comments are used in special cases (check the codebase to see when to use them)
  • You committed an empty json config file, it's better to commit a file with some defaults so a reviewer can get an idea of what it will be used for

Comment thread code/common/getter.q Outdated
@bsofjan

bsofjan commented Jun 25, 2026

Copy link
Copy Markdown
Author

I have reviewed the TorQ codebase and could not find an existing utility function for the setter.q and getter.q functions. There are existing save and load utilities in the TorQ codebase, however they are highly specific (e.g. loading q and k scripts, saving partitions) and not suitable for use for saving and loading with the anymap data format.

I have also addressed all the other points through my latest commit "Standardising Changes and Add Default json" and by adding more detailed description to the PR.

@CJA18

CJA18 commented Jun 25, 2026

Copy link
Copy Markdown

@bsofjan thanks for checking if the utility was present in TorQ and for addressing my other comments - looks good to merge

@bsofjan

bsofjan commented Jul 6, 2026

Copy link
Copy Markdown
Author

@CJA18 Thank you - I think it is best to wait prior to merging as the PRs functions as checkpoints of work, and merging can be considered once the whole project comes around.

Comment thread code/common/getter.q
\d .getter

// Get cache from disk
getcachefromdisk:{[filePath] load hsym filePath}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

filePath -> filepath

this also seems like a very thin wrapper around load, is it really useful to have this?

Comment thread code/processes/setter.q
\d .setter

// Save cache down to disk
savecachedowntodisk:{[data;filePath] (hsym filePath) set data };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

filePath -> filepath

and similar to the getter function, very thin wrapper around set, is it useful?

Comment thread code/processes/setter.q

// Get location of cache config and load it in.
cacheconfiglocation:.proc.getconfigfile["cacheconfig.json"];
cacheconfig:.j.k raze read0 hsym first cacheconfiglocation;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think .proc.getconfigfile should only return one file, do we need the first here?

Comment thread code/processes/setter.q
@@ -0,0 +1,12 @@
// Setter process to set cache to disk

\d .setter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this namespace is unclear, without having additional context it isn't apparent what "setter" is or is for

Comment thread code/processes/setter.q
@@ -0,0 +1,12 @@
// Setter process to set cache to disk

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should this be a standalone process? seems like caching would be something that multiple processes would do so perhaps this should be a lib rather than a process?

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