#Supporting Context Directories

1 messages · Page 1 of 1 (latest)

hushed musk
#

cc @magic matrix

polar plank
#

Hey, you need to add more metadata do function arguments, in particular these two:

  • DefaultPath(): taking a string
  • Ignore(): taking a list of strings

In Function.withArg, you have options to pass those values in.

magic matrix
#

so two attributes?

hushed musk
#

Ah I see now.

So two attributes as @magic matrix suggests, and when we find them, pass them in here:

    /**
     * Returns the function with the provided argument
     */
    public function withArg(
        string $name,
        TypeDefId|TypeDef $typeDef,
        ?string $description = '',
        ?Json $defaultValue = null,
        ?string $defaultPath = '',
        ?array $ignore = null,
    ): Function_ {
#

Thank you @polar plank

polar plank
#

Yes. Could be a single Argument() attribute, with multiple options, like TypeScript. Python uses different ones because there was already a Doc() for descriptions so it was weird for it to stand outside.

hardy oar
hushed musk
#

@hardy oar I've tried setting up context directories here: https://github.com/dagger/dagger/pull/8411

It either is not quite working, or it is not working in the way I expected it to.

I set a defaultPath on a directory:

    public function test(
        #[Argument('Run tests from the given source directory')]
        #[DefaultPath('../')]
        Directory $source,

When I check the help for that function I see this:

--source Directory   Run tests from the given source directory [required]

Naturally if I try dagger call test I get an error saying:

Error: required flag(s) "source" not set
GitHub
feat: Support DefaultPath and Ignore in PHP SDK by charjr · Pull Re...

Add support for default path and ignore in modules created using the PHP SDK.

#

@hardy oar I make it optional, like so:

    public function test(
        #[Argument('Run tests from the given source directory')]
        #[DefaultPath('../')]
        Directory|null $source,

--source Directory   Run tests from the given source directory

But then I get the following:

[ERROR] DaggerModule\PhpSdkDev::base(): Argument #1 ($source) must be of type  
         Dagger\Directory, null given, called in
polar plank
#

You may need to add the WithOptional(true) but the engine should enforce that rather then SDKs.

hushed musk
#

$typeDef = dag()->typeDef()->withOptional($type->nullable)

#

So I may be overriding the engines behaviour by explicitly setting withOptional(false) when it's not nullable.

#

That may be part of the issue so thank you for pointing that out. I will have to change that behaviour as well.

But it still doesn't solve the issue of why, even when I enforce it as optional, the DefaultPath isn't used

polar plank
#

You need to pass withOptional(true) even if the argument's type is non-null, in case DefaultPath is used.

#

But it still doesn't solve the issue of why, even when I enforce it as optional, the DefaultPath isn't used

I presume you don't get the chance to see it being used if the argument is being reported as required.

hushed musk
#

Right so something along the lines of this?

if (type is nullable OR type has DefaultPath) {
    set withOptional(true)
}
hardy oar
#

But I agree that it should be handled by the engine instead

hushed musk
hardy oar
#

It's not properly handled in the engine so yep, are you doing the same @polar plank ?

polar plank
#

Yep

hushed musk
#

I'll sort out the withOptional feedback for now and see if that gets me anywhere, thank you both for the help 🙂

hushed musk
hardy oar
hushed musk
hushed musk
hardy oar
#

I'll check that!

#

Pr checked 😄

hushed musk
# hardy oar Pr checked 😄

So the issue is in PHP:

If I have an argument like this:

public function myDaggerFunc(Directory $dir)

I cannot give a simple default value like this:

public function myDaggerFunc(Directory $dir = '.')

It would throw a TypeError because a string is not a Directory

#

So if I wanted to have a default value in the normal way a PHP developer would do it. I would actually have to create a Directory by scratch, without using the dagger client

#

Unless there's an easy way for me create a context directory as an object?

#

I'm only elaborating here to see where the misunderstand lies:

Am I not clearly explaining the difficulty in the README or am I not understanding the full capacity of the context directory?

polar plank
# hushed musk Unless there's an easy way for me create a context directory as an object?

There's no "context directory" object. It's just a normal dagger.Directory at runtime. The defaultPath setting should be an argument in FunctionArgument(defaultPath: '.'). It's metadata. Arguments with a defaultPath should not have a default value. For example, this will return an error:

@function
def my_dagger_func(self, source: Annotated[dagger.Directory, DefaultPath(".")] = DEFAULT_DIRECTORY) -> dagger.Directory:
  return source
hushed musk
polar plank
hushed musk
hardy oar
hardy oar
hushed musk
#

@true sierra @polar plank I've updated it once more. I think I've addressed all feedback. Thank you for the multitude of reviews 🙂