jsonnet icon indicating copy to clipboard operation
jsonnet copied to clipboard

Deprecate std.thisFile

Open sbarzowski opened this issue 5 years ago • 9 comments
trafficstars

If some path is needed for the calculation, extVar or TLA should be used instead.

The way it currently works leaks information to Jsonnet. In particular it becomes important whether a file to evaluate was passed as a relative or an absolute path.

sbarzowski avatar Aug 07 '20 18:08 sbarzowski

I've found the values that this field takes on to be surprising, and hard to predict. The main use I've had for it it is subjecting it to a basename function. The file name is usually reasonable; it's the parent directory path that winds up barely usable.

seh avatar Aug 07 '20 20:08 seh

While I agree with the sentiments above, I have one crappy native function in qbec that depends on std.thisFile being available. The user is expected to pass std.thisFile as an option to the function so it can load relative paths from the filesystem correctly.

Let's set aside the implementation crappiness of this particular function for this discussion :)

The general problem is the inability for native functions to read files from paths relative to the file where the function is invoked. If it could be arranged such that native funcs could resolve such paths correctly, that would be great.

gotwarlost avatar Aug 08 '20 14:08 gotwarlost

Thanks for providing your perspective. It's always useful to get feedback about how the functionality is actually used.

First let me clarify that deprecate != remove. For now I just want to make it clear in the docs that it is not recommended, add warnings to the linter (after it is released...) and give a mandatory sermon to everyone who has problems related to it. It certainly won't stop working without warning. We have plenty of ideas to improve Jsonnet without breaking the users :-).

The general problem is the inability for native functions to read files from paths relative to the file where the function is invoked.

First, using a native function to read files (or do any IO) is already an abuse of the API. It was supposed to be used for pure functions which are too complicated to reimplement or require better performance than can be achieved in Jsonnet.

I understand that sometimes a pragmatic trade-off needs to be made and it's not as bad if you can give an illusion that it's a pure function (i.e. that the result depends only on arguments and there are no side effects). Your proposal to make it aware of the current file goes contrary to that – calling that function with exactly the same arguments in a different file could produce a different result.

If you want we can discuss better ways to implement this functionality, feel free to open another issue. Here, it's out of scope.

sbarzowski avatar Aug 08 '20 17:08 sbarzowski

@gotwarlost You could probably use an import handler instead of a native function for that

sparkprime avatar Aug 09 '20 20:08 sparkprime

@sbarzowski thanks for the feedback. I didn't assume that deprecate == remove BTW. I'm also fully aware that I'm abusing the API with that native function.

I was just using this issue as an opportunity to discuss non-standard "stuff" that we've been doing and converge on something better by talking to y'all.

(Maybe I should create a new issue for this discussion)

@sparkprime thanks for the import handler idea, I'll look into that approach.

gotwarlost avatar Aug 10 '20 16:08 gotwarlost

As another data point on usage in the wild, we use std.thisFile in our grafana dashboards to construct a link to the origin file in github in the resulting dashboard: example edit link button

More generally, it can be useful information to include in the final output for debugging or tracking purposes.

ekimekim avatar Aug 22 '20 01:08 ekimekim

I wonder if it would be more appropriate to provide separate file and path, and the path would always be absolute. That would go hand-in-hand with sandboxing file access according to a virtual filesystem created from the CWD and -J paths, and possibly some updates to the import handlers to separate the relative path resolution from the actual I/O. Just some thoughts.

sparkprime avatar Aug 22 '20 12:08 sparkprime

It would be nice to document when thisFile evaluates to a relative path vs an absolute path; I have observed both (depends on the dir I invoke grr, as part of the Grafonnet workflow).

sfc-gh-bxin avatar Nov 11 '21 01:11 sfc-gh-bxin

Adding my POV: this is the reason I've come to jsonnet in the first place. I've looked after something, which can automatically create context-sensitive templates for Kubernetes objects inside of Argo CD and because Kustomize is not powerful enough, I found Argo CD's jsonnet support to perfectly fit.

My use case is the following: I have various projects/resources, for which I need to create templated Kubernetes resources based on their file system path (elements from the path), so the templates get autofilled/customized according to the file system path, with a manner of a jsonnet import. I haven't yet found a cleaner and nicer way of doing this and this solution is entirely based on the ability to get the actual path for the file in question.

Please don't deprecate/remove this feature!

bra-fsn avatar Jun 23 '23 16:06 bra-fsn