#Private properties in go/python/typescript

1 messages ยท Page 1 of 1 (latest)

untold summit
#

So currently, private properties in go work using // +private - essentially, these are properties that are on an object that are part of the object, but not part of the exposed api

#

Now this just means we don't generate the typedefs. But for, sooooo many reasons, we can't keep doing this, we need to actually expose everything - this is because, if you put an ID in a private property atm, the engine "loses" it, and can't find it again

#

But... I've run into the problem that, at least in go, some users have put things like maps behind // +private, as a way to hack it to get it to work - which it does

#

so i'm kind of struggling to understand if python/typescript users might have done something similar? and also have difficult it would be to expose everything to the typedefs that's currently not?

dusky canopy
#

Hmmm... in Python and TypeScript, the syntax is different but the functionality is the same. There's use cases for having state that's not exposed as a function. On the SDK's side I can validate whether you're attempting to serialize a dagger object that's not exposed. There can be complex data structures in there but otherwise serializable by the SDK, that would be hard to try and register it all in the API.

untold summit
#

i feel like part of the problem, is that // +private does two things - it hides something from the api, but also hides it from the engine

#

i wonder if we could split it into two separate functionalities? // +private hides from the public api, and // +internal or something keeps it entirely internal to the module (but not sure how that translates to not-go sdks)

untold summit
#

would we error out if you tried to do that? or maybe return them as "excess" return data, so we can track the ids?

#

i have a "hack" that kind of fixes the original issue, by looking for anything "id-like" in the json directlry, while having no idea of what it's inspecting, but ๐Ÿ˜„ obviously there's no guarantee this works

dusky canopy
#

Hmm... thinking about the Discovery // +private in the Python runtime, that structure actually has a dagger.Directory and a dagger.ModuleSource. In that case I don't want them to be exposed on the API. ๐Ÿค” But maybe it wouldn't be too bad to force me to remove // +private from there and add it to all the Discovery fields that aren't dagger objects.

untold summit
#

yeah, the problem is i actually really like the idea of having internal details using json-serializable stuff in your own language, e.g. like datetime in go could be a really cool use of it

#

here's a maybe idea:

  • // +private gets split into // +internal and // +private
    • for python, this would probably have a special new private_field or something, with internal being the "default"
  • then, in each sdk, if you try and put a dagger object in a non-annotated field (e.g. internal), then you get told off and error out
dusky canopy
#

Does it create issues with only some types or is it everything, include dagger.Directory? Do you know?

untold summit
#

technically, we just see this with Secret/Socket - but also PBDefinitions doesn't work with this either, it's been a known "bug" for a while, but I guess it just never surfaced into something observable

#
// If we didn't do this, then it would be possible for Buildkit to prune the
// content pointed to by the blob:// source without pruning the function call
// cache entry. That would result callers being able to evaluate the result of
// a function call but hitting an error about missing content.
untold summit
#

bleh, i kinda wish we hadn't ignored this case when we added private fields in the first place

grim talon
#

What if we expose it to the schema but add a private property in so it's skipped by the generator?
Would this fix your issue?

The engine would be aware of it but not the user

untold summit
#

yeah, i've done this ๐Ÿ™‚ but the problem is that you start hitting types that dagger doesn't know how to expose in the engine at all (like maps, etc)

grim talon
#

Oh yeah that's so right

#

We should support these types in the long run though

#

But that's much more work

untold summit
#

mm, but there's a lot of types ๐Ÿ˜›

grim talon
#

Is there a way to create a generic "any" type when you just need a way to jsonignify it

untold summit
#

i tried hacking this with a new UnknownType when you hit something in go that you don't know how to handle

grim talon
#

So the engine can handle "any" as generic json?

untold summit
#

but this is still an issue. because the engine actually needs to know it's json structure, or it can't look through it

grim talon
#

Yeah that's why I'm saying we could have a "any" type which must be a JSON at least

#

So we could apply this for private fields

untold summit
#

right yes - but how do you know which things are ids in there? ๐Ÿ™‚

grim talon
#

Hmm

untold summit
#

i have a little hack to just "guess"

#

but this doesn't feel right at all

grim talon
#

You recursively explor the json object to look for IDs but yeah that's bad

#

We should try to support more and more types until we cover all cases

#

Map should be fairly similar to object

#

A map is an object basically but without funcs

untold summit
#

๐Ÿค” i have thought about the idea of a custom serialization format before. if we could have a way of being like "this is definitely an id", then the problem is solved

grim talon
#

True

untold summit
#

maybe we could just prefix a magic prefix to string id fields?

#

and have an escape char if a literal actually has that

grim talon
#

Like what? ๐Ÿ˜ฎ

untold summit
#

so id(container) = "id:...." in json

#

or even id(container):... would be better

#

then you know what type the id is of as well

#

pretty sure this would solve the issue actually ๐Ÿค” for all cases that don't do insane json serialization (e.g. super contrived example, if something did json serialization by first serializing to pickle or something, and then making a base64 encoding of that)

grim talon
#

Would be cool yup

untold summit
#

adding nice id prefixes is easy enough, we could definitely have that

#

but working out how to distinguish those from just normal strings is still not super easy

#

๐Ÿค” maybe we could encode ids, not as strings, but as an object, with something like an empty key: e.g. { "": "<id>" }