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

Auto watch jpath #575

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

Conversation

wilfriedroset
Copy link

The jpath contains additional library used to manifest the dashboards. During the development workflow one might modify the underlying lib. This commit makes grr automatically watch the jpath. Moreover, a change in the jpath will trigger the reload of all watched files.

Fixes: #555

The jpath contains additional library used to manifest the dashboards.
During the development workflow one might modify the underlying lib.
This commit makes grr automatically watch the jpath.
Moreover, a change in the jpath will trigger the reload of all watched
files.

Signed-off-by: Wilfried Roset <[email protected]>
@wilfriedroset wilfriedroset requested a review from a team as a code owner January 27, 2025 10:25
@Pokom
Copy link

Pokom commented Jan 28, 2025

Thanks @wilfriedroset for the PR! I've pulled and tested locally and think it looks reasonable. I'm going to ask @K-Phoen to take a peak as well as they have a bit more context into this part of grizzly.

I've allowed the PR to run relevant workflows as well, so you can get results from CI.

@malcolmholmes
Copy link
Collaborator

A few comments. Grizzly started as a Jsonnet tool, hence Jpath being an important thing. But it evolved, with Jsonnet becoming only one way of interacting with it - no other way requires jpath.

Therefore, I'd prefer it if the "blast radius" of the jpath could be limited - if there's a watchPaths, can't the jpath dirs just be added to that right from the get go, then the rest of the code needn't change (and can stay oblivious of jpath's existence)?

@wilfriedroset
Copy link
Author

if there's a watchPaths, can't the jpath dirs just be added to that right from the get go

This is what I initially though and tested. This is how I stumble upon the unexpected behavior I've reported here: #555

My understanding is that the use of watchPaths to watch both the mixins (e.g: dashboards) and the lib cannot work as it is now. Indeed, when grizzly catch a change in the lib and not the mixins themselve, grizzly tries to manifest/build the lib which produce the error RUNTIME ERROR: couldn't manifest function as JSON

This is why I've come up with the following PR. Another idea I add while I was working on it was to add another flag, watchPathsWithoutManifests and any changes in those path would trigger the manifest/build of anything in watchPaths.

My understanding of grizzly is not as deep as I would like hence I would like your guidance @malcolmholmes about the next course of actions to fix #555

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.

Failed to manifest dashboard when modifying lib
3 participants