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

Refactor xapi-storage-script to use modules #6191

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Vincent-lau
Copy link
Contributor

The bind function in xapi-storage-script has lots of implementaions all over the place, making it hard to maintain. Use module abstractions to separate different storage functions.

The tricky bit of this is the need to pass version and volume_script_dir into each storage function calls, and these two variables are determined at runtime. Hence functors are used for this purpose, once the volume_script_dir is determined when bind is called, pass this as inside the RuntimeMeta module to the relevant implementations. The version global variable, however, is populated when Query.query is called, so create an alias in the RuntimeMeta module so that it can be used in the storage function implementations.

The `bind` function in xapi-storage-script has lots of implementaions
all over the place, making it hard to maintain. Use module abstractions
to separate different storage functions.

The tricky bit of this is the need to pass `version` and
`volume_script_dir` into each storage function calls, and these two
variables are determined at runtime. Hence functors are used for this
purpose, once the `volume_script_dir` is determined when `bind` is
called, pass this as inside the `RuntimeMeta` module to the relevant
implementations. The `version` global variable, however, is populated when
`Query.query` is called, so create an alias in the `RuntimeMeta` module
so that it can be used in the storage function implementations.

Signed-off-by: Vincent Liu <[email protected]>
let module RuntimeMeta = struct
let volume_script_dir = volume_script_dir

let version = version
Copy link
Member

@psafont psafont Dec 20, 2024

Choose a reason for hiding this comment

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

If I understood correctly, version is shared among all the modules that implement storage functions. Why is it needed in RuntimeMeta? It looks like it depends on the global variable anyway, so why bother with the indirection?

For volume_script_dir it makes sense because the value is passed when calling bind, but for version, the value is set after calling bind.

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.

2 participants