chore(sdk/php): use DefaultPath to SDK s... | Dagger | Page 1

near haven Jan 21, 2025, 4:41 PM

#

Thank you, LGTM!

I wanted to do this recently but we had to add that Container argument during this PR.

I am curious if, since the PhpSdkDev got unpinned do we actually need this argument anymore?

cc @silver flicker hopefully not too much of a bother, I'd just like to pinch your sagely dagger wisdom on this one 🙏

GitHub

ci: un-pin php sdk to use same version as engine tag by jedevc · Pu...

We were pinned to an old version of the php sdk that didn't have the proper OTEL work that allowed us to actually observe errors happening.
By unpinning it, we'll always get the sam...

GitHub

feat(engine): support expanding variables in more functions by raja...

This adds expanding environment variables in WithNewFile. ref: #7951. If this looks OK, I will add similar changes to other API's in this same PR.

#

Actually there's even one weird edge case that is guaranteed to confuse people if we have both arguments:

If someone passes a Source directory and also they pass a Container. The outcome will be that Source is completely ignored and only the Container will be used.

near haven Jan 21, 2025, 5:05 PM

#

This edge case could occur right now, still. So it doesn't block this PR.

I just noticed it now though and if we could solve it, that would be nice

keen zinc Jan 21, 2025, 5:39 PM

#

If someone passes a Source directory and also they pass a Container. The outcome will be that Source is completely ignored and only the Container will be used.
Oh, would it be Test function that can trigger this issue? It use phpBase from the root dagger module and pass it to the dev module.

#

Reference from Python implementation https://github.com/dagger/dagger/blob/main/.dagger/sdk_python.go#L98

If we expose a container from the dev, perhaps it could be solve this issue?

GitHub

dagger/.dagger/sdk_python.go at main · dagger/dagger

An engine to run your pipelines in containers. Contribute to dagger/dagger development by creating an account on GitHub.

silver flicker Jan 22, 2025, 10:46 AM

#

near haven Thank you, LGTM! I wanted to do this recently but we had to add that Container ...

Yes, this is still required

#

So, the *Container is the dev version of the engine that has been created

#

However, when unpinning, that just matches the version of the engine that runs the tests

#

So suppose I'm running PHP tests on main using dagger v0.15.2. Then I want to test against main engine not v0.15.2 engine

near haven Jan 22, 2025, 10:54 AM

#

Right I see. So to clarify, do all other SDKs also have a way to pass in the *Container?

silver flicker Jan 22, 2025, 10:59 AM

#

yup, all the dev sdks do

#

eventually we'll remove it from all of them - and they'll just be able to access it by installing an engine dependency

#

but there's some work to get there

near haven Jan 22, 2025, 11:02 AM

#

That's fair enough, I'll try to keep an eye out for when we can do that 👀

near haven Jan 22, 2025, 11:29 AM

#

Either way it's not something that affects users and it's not new to this PR so I approved the changes.

GitHub

feat(sdk/php): support float type by wingyplus · Pull Request #9411...

This PR follow up #9396.
How to test this feature
I create a module named potato and put the following code to src/Potato.php:
<?php

declare(strict_types=1);

namespace DaggerModule;

use D...

silver flicker Jan 22, 2025, 1:08 PM

#

wooo thanks ❤️

#chore(sdk/php): use DefaultPath to SDK s...