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.

Sign up for GitHub

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

Jump to bottom

Add static data support to bundler #179

Merged
merged 12 commits into from
Oct 10, 2024
Merged

Add static data support to bundler #179

merged 12 commits into from
Oct 10, 2024

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
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

garryod
garryod reviewed Oct 3, 2024
View reviewed changes
Copy link
Member

@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

bundler/src/main.rs Outdated Show resolved Hide resolved
static/admin.json Outdated Show resolved Hide resolved
bundler/src/bundle.rs Outdated Show resolved Hide resolved
bundler/src/bundle.rs Show resolved Hide resolved
@tpoliaw
Review fixings
cf40d51 
* Unbox CLI args
* Include static data in revision info
* Fail on fs errors when reading static data
@tpoliaw
Set static data via globs instead of directory
546ea0e
garryod
garryod requested changes Oct 3, 2024
View reviewed changes
Copy link
Member

@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

bundler/src/main.rs Outdated Show resolved Hide resolved
bundler/src/bundle.rs Outdated Show resolved Hide resolved
@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
View reviewed changes
Copy link
Member

@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

@tpoliaw
Add beamlines to em_admin and epsic_admin groups
17b5efb 
These are the most common beamlines for visits of users with
the respective admin permissions.
@tpoliaw
Move static data to bundler helm charts
f19b704
@tpoliaw
Store bundler static data globs as strings
3735d05 
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.
garryod
garryod reviewed Oct 9, 2024
View reviewed changes
bundler/src/main.rs Outdated Show resolved Hide resolved
@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
Member

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

@tpoliaw
Remove static bundler data
ec4a444 
This can be added at a later time and is not required for static data
support.
garryod
garryod reviewed Oct 9, 2024
View reviewed changes
bundler/src/main.rs Outdated Show resolved Hide resolved
garryod
garryod approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@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
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garryod garryod garryod approved these changes

Assignees

@tpoliaw tpoliaw

Labels
enhancement New feature or request rust Pull request that updates Rust code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tpoliaw @garryod