#I hit the original issue because I did
wraith pecan
@lunar pelican
I can push the changes on my PR if you want to have a complete look
lunar pelican
Constructor seems to be called twice in python
wraith pecan
So you'll have the same issue if you try something like I did
Or maybe not since dagger will save the constructor result on the first call
In Python, if you have a field, do you automatically have a constructor or it's not like that?
wraith pecan
Oh it's not like that in Ts
I mean, the constructor will be called by dagger only if it's registered (so the function must exist)
I'm calling the constructor to instanciate the class but not anymore because it fails if there's required argument
If you want to test it:
- Create a constructor in python with a required argument (not related to a field)
- Create a function
- Call the function with dagger call
You might trigger an error when you call the constructor for the function
lunar pelican
It's only a problem if you compose the inputs with something more before saving.
wraith pecan
It's only a problem if you compose the inputs with something more before saving.
What do you mean?
lunar pelican
Post init is mutating the input "foo" here:
from dagger import function, object_type, field
@object_type
class Testme:
foo: str = field()
def __post_init__(self):
self.foo += "bar"
@function
def echo(self) -> str:
return self.foo
So when I dagger call --foo=hello echo, I'll get "hellobarbar".
If I don't provide --foo, it should error as expected.
wraith pecan
I meant if you have a constructor but not related to a field
For example in Typescript
@object()
class A {
ctr: Container
constructor(src: Directory) {
this.ctr = dag.container().withDirectory("/", src)
}
@func()
async test(): Promise<string[]> {
return this.ctr.directory("/").entries()
}
}
In this example, src isn't a field, it's just a constructor argument
lunar pelican
There shouldn't be a problem in Python in this case, let me check.
wraith pecan
If your constructor is called 2 times, that means when you'll call test, you'll create the class but without the constructor argument and it will fails?
Dagger will do -> call constructor -> get state -> call test with state
So you'll create your class on time, return to dagger, and then another time to call test no?
lunar pelican
No, Python has two constructors 😈
wraith pecan
What?
lunar pelican
This situation is actually tested.
lunar pelican
It's simple, Python supports a factory, which can be used as the constructor in the API:
from dagger import function, object_type, field
@object_type
class Testme:
foo: str = field()
@classmethod
def create(cls, bar: str):
return cls(foo=f"{bar}!!!")
@function
def echo(self) -> str:
return self.foo
It was necessary to support async operations in the constructor, but it also supports your use case. So the first invoke (name: ""), create is called, and it returns an instance of Testme, with the computed value of foo. The second one has that result in parentJson, so the default constructor is called (Testme(foo="hello!!!")), and then echo is called on this instance.
wraith pecan
So that's the trick...
So if I want it to work, I need a special function that will be the user constructor.
And I leave the new MyClass() alone
lunar pelican
But I do think it's worth pursuing not executing the constructor the second time, and just attributing the fields. Tricky though, at least without some metaprogramming.
wraith pecan
I'll try another way -> bind a default constructor with empty argument if no one is defined by the user
Using overloading logic -> new Foo("a") works but also new Foo()
That way, it will not fails if the class is instanciate without arguments
It seems I cannot do that kind of trick
@lunar pelican Would it be okay if I add a decorator @constructor and call it when the constructor is called by dagger (like you do)
For example
@object()
class A {
@constructor()
init(): A {
... Logic here
}
}
This would fix all my issue
lunar pelican
You'll need to check if the decorator is used multiple times in the same object 🙂
wraith pecan
Uuuugh
Other idea: just let user know that they might need to declare an empty constructor by default if they have field assigned
Just so it's called by dagger before calling any other func
If I add it inside the default template, it's not much a deal
It doesn't limit the user in its experience, it's simply that if a user wants to use the default property field notation, he still needs to declare a constructor so properties are correctly assigned (works even if the constructor is empty)
Example:
class Foo {
a = 4
b = 5
}
Becomes:
class Foo {
a = 4
b = 5
constructor() {}
}
That's not good dx. Isn't it the problem when you need a constructor arg that's not a field to produce the value of a field?
That's a factory.
wraith pecan
Nope
wraith pecan
That's not good dx. Isn't it the problem when you need a constructor arg that's ...
The issue is: #daggernauts message
Basically, in case the constructor is called: I use new, otherwise Object.create but Object.create does not init properties so it's a problem if the constructor has never been called by Dagger because values are not stored in the dagger state in that precise case
lunar pelican
But if parentJson isn't empty then surely it's been through the constructor already.
lunar pelican
Why can't you just attribute the values from parentJson to the fields directly? Without having to go through the constructor again?
wraith pecan
Why can't you just attribute the values from `parentJson` to the fields directly...
Oh I can, but if there's no constructor, then parentJson is empty
wraith pecan
But again, what if your constructor has as default argument something that is not a field
lunar pelican
That's what you get in parentJson -> fields
wraith pecan
Hmm
lunar pelican
But again, what if your constructor has as default argument something that is no...
Then it will be called on the first invoke (for name: ""), but not the second.
wraith pecan
Wait that means I have some typescript tests that are wrong
@registry.object()
class HelloWorld {
prefix = "Hello"
@registry.func()
greeting(name: string): string {
return `${this.prefix} ${name}`
}
}
const result = await registry.getResult(
"HelloWorld",
"greeting",
{},
{
name: "world",
}
)
assert.equal(result, "Hello world")
})
This cannot happens
prefix will be "Hello" in the parents when called by dagger, right?
lunar pelican
Looks fine, result should be "Hello world".
Yeah, from Dagger, that parentJson won't be empty.
But it'll have {prefix: "Hello"}, which is the same as the default. So same result.
wraith pecan
Not for me, because the constructor since it doesn't exist isn't called so default values needs to be passed by dagger
wraith pecan
It's a unit test, so I'm not using dagger here
lunar pelican
Yeah, that's why we need integration tests.
wraith pecan
If I put it in dagger call it will work, because parent will not be empty
lunar pelican
Your assumptions about the API could be wrong.
lunar pelican
If you say the constructor won't be called, then how is parentJson not empty?
wraith pecan
If you say the constructor won't be called, then how is `parentJson` not empty?
Because Dagger provides the default state?
lunar pelican
Yeah, true.
wraith pecan
Ok, I need to update my tests to take that behaviour into account
That's why my unit tests failed but not integration tests
Love it ^_^
lunar pelican
I had a discussion about that with Erik because that doesn't feel right from the language's point of view. I didn't want the API to set and send the default value.
wraith pecan
In that case, an idea came into my mind
I can call the constructor if the state is empty and the called function isn't the constructor.
Because that means the constructor does not exist (because no state has been pushed)
The only edge case is if you have a constructor that doesn't set any state but what the point of a constructor then
---> Function that isn't the constructor
---> Do we have a state?
-> Yes = Object.create (a constructor has been called before)
-> No = new ()
This could work and cover the case you talk about with Erik
lunar pelican
No not really. The case was having a state but that state being the default. The API sets and sends that value to the constructor.