Skip to content

Add static data support to bundler#179

Merged
garryod merged 12 commits intoDiamondLightSource:mainfrom
tpoliaw:static_bundle_data
Oct 10, 2024
Merged

Add static data support to bundler#179
garryod merged 12 commits intoDiamondLightSource:mainfrom
tpoliaw:static_bundle_data

Conversation

@tpoliaw
Copy link
Collaborator

@tpoliaw tpoliaw commented Oct 2, 2024

Proof of concept for adding static file support to bundler service. Any files named *.json in the optional static_data_directory will be included in the generated bundle under a name matching the file name.

eg, using the new static directory, the bundle structure is now

.
|-- .manifest
`-- diamond
    `-- data
        |-- admin
        |   `-- data.json
        |-- beamlines
        |   `-- data.json
        |-- proposals
        |   `-- data.json
        |-- sessions
        |   `-- data.json
        `-- subjects
            `-- data.json

Static directory location subject to bikeshedding.

@tpoliaw tpoliaw force-pushed the static_bundle_data branch from 8d95079 to 08b1d8e Compare October 2, 2024 16:50
@tpoliaw
Copy link
Collaborator Author

tpoliaw commented Oct 2, 2024

I also have no idea which beamlines belong to which group so the sample admin.json is very incomplete

@tpoliaw tpoliaw force-pushed the static_bundle_data branch from 08b1d8e to c63aa93 Compare October 3, 2024 10:00
@tpoliaw tpoliaw force-pushed the static_bundle_data branch from c63aa93 to 1bda715 Compare October 3, 2024 10:14
Copy link
Contributor

@garryod garryod left a comment

Choose a reason for hiding this comment

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

Looks reasonable, think I'd rather pass a glob in than have it select every json in a directory

* Unbox CLI args
* Include static data in revision info
* Fail on fs errors when reading static data
Copy link
Contributor

@garryod garryod left a comment

Choose a reason for hiding this comment

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

Looking good, just a couple little bits

@garryod garryod added enhancement New feature or request rust Pull request that updates Rust code labels Oct 3, 2024
garryod
garryod previously approved these changes Oct 3, 2024
Copy link
Contributor

@garryod garryod left a comment

Choose a reason for hiding this comment

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

LGTM

These are the most common beamlines for visits of users with
the respective admin permissions.
They are used as strings so storing them as strings makes more sense. It
also makes the tracing sane again as debug format for patterns is
incredibly verbose.

Strings are still validated by the CLI.
@tpoliaw
Copy link
Collaborator Author

tpoliaw commented Oct 9, 2024

The chart lints are failing, presumably because adding the static data counts as a change. Should the data be added in its own PR?

@garryod
Copy link
Contributor

garryod commented Oct 9, 2024

The chart lints are failing, presumably because adding the static data counts as a change. Should the data be added in its own PR?

Yeah, probably add it as it's own thing - or as part of #186

This can be added at a later time and is not required for static data
support.
Copy link
Contributor

@garryod garryod left a comment

Choose a reason for hiding this comment

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

LGTM

@garryod garryod merged commit d6dbcab into DiamondLightSource:main Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request rust Pull request that updates Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants