#feat: add contextual git by jedevc · Pul...

1 messages · Page 1 of 1 (latest)

eager pasture
ornate turtle
#

yeah 😭 I'd forgotten that point

#

sad, but I'll live with the default*

eager pasture
#

So, I see a few options appearing more clearly..

#
  1. We follow the path of least resistance, and add +defaultGit, +defaultContainer, etc
eager pasture
ornate turtle
#

defaultAddress would work nicely imo, though I don't love the naming 🤔

#

no better ideas tho lol 😆

eager pasture
ornate turtle
#

mm 🤔 problem is this is now blocked until tom and helder are back

#

I like 3 - but can't speak with enough authority on how possible that is

#

(it also wont work for java fyi)

eager pasture
#

I'll just vibe-code it!

#

(joking)

ornate turtle
#

ahahaha I was about to say :p

eager pasture
#

@rotund cloud you have some decent context on the SDKs, maybe you have a gut feeling on feasibility?

ornate turtle
#

um, do we wanna unblock this feature and go with path of least resistance? it's not effort to keep supporting even if we go to another route later

#

given that this discussion needs a bit of a time I think

#

i really want to use this to give us a big speed boost in our version module

eager pasture
#

@ornate turtle as a stopgap, how would you feel about allowing +defaultPath to apply to GitRef and GitRepository? That way we don't cross any one-way doors on the API side

ornate turtle
#

mmmmmm not my favorite honestly 🤔 it's still a bit of a one way door, cause we'll need to support that syntax indefinitely

eager pasture
#

And conceptually it's aligned with option 2 (minus the rename)

eager pasture
ornate turtle
#

yeah if it's a choice between those two options, we should just delay until we get consensus

eager pasture
#

We support remote git & local paths in all sorts of places. dagger -m, .cd...

ornate turtle
#

I don't want to overload the option we purposefully narrowed

eager pasture
#

So it's not reverting a past decision - more like addressing a blind spot of the original decision

ornate turtle
eager pasture
ornate turtle
#

no no

#

defaultGit=... would work

#

defaultPath only works with local paths

eager pasture
#

But as a user I would expect defaultPath= to work that way today, to be consistent with our CLI

ornate turtle
#

fair, if only there was a pr that was working to resolve that 😛

eager pasture
#

There is?

ornate turtle
#

isn't that what address is all about

eager pasture
#

Oh right. Yes.

#

🙂

ornate turtle
#

i guess if that's the future we're going for

#

i'm okay with overloading defaultPath

eager pasture
#

My point is, even before expanding past Directory and File, I think there's a very strong argument that +defaultPath can naturally support git paths, and users won't be surprised or unhappy.

ornate turtle
#

but we would have a temporary inconsistency

eager pasture
#

(since we set the precedent that directories can be loaded from git addresses)

ornate turtle
#

yeah okay, that's fair to me, i'm happy with that compromise

eager pasture
#

How about this:

  1. We quietly expand defaultPath to support all address-loadable object types
  2. Later we find a better alternative to defaultPath (either option A or B below ) and soft-deprecate it
  3. Developers slowly adopt the better alternative. We probably continue supporting defaultPath, even deprecated, for a long time (if only in compat mode)

Possible alternatives to +defaultPath:

  • Option A: semantic change only. Rename +defaultPath to +defaultAddress, +defaultObject, +defaultRef, or another name that makes it clear all address-loadable object types are supported.

  • Option B: Python and Typescript always use native default arguments, using the new address() API. eg. func foo(source = dag.address(".").directory()). In Go, rename +defaultPath to +default

ornate turtle
#

i would say we can expand but only for path-like things

#

e.g. we shouldn't support it for things that aren't paths, we should just wait to add those until we have a better option

#

shouldn't be an issue, i can't think of any of those off the top of my head we want to add (rn)

#

@eager pasture any other comments r.e. that pr? will probably context switch back to it today, but wanna make sure i get as much fixed as possible

rotund cloud
#

Why not something like defaultRef (regarding naming of defaultAdress or other alternatives)? A reference can be local, remote, a directory or git, etc
This might sound a little bit less obvious for a local path, but if it encapsulates multiple types (dir, file, git, etc) and multiple provenances that might cover it

eager pasture
#

What we're also curious about @rotund cloud is "option B" which would be more ambitious (not just a rename of the pragma):

Python and Typescript always use native default arguments, using the new address() API. eg. func foo(source = dag.address(".").directory()). In Go, rename +defaultPath to +default

Last year that seemed to be difficult, because of engine and SDK internals, but it was seriously considered. Maybe now it's easier?

One big difference is that now (with the address() PR) there already is an API to call

#

@ornate turtle back to the PR, why go through defaultGit in the first place, instead of eg. dag.CurrentModule().GitRepository() ?

ornate turtle
#

aha

eager pasture
#

Not that I'm not enjoying the rabbit hole conversation that led to 🙂

ornate turtle
#

blueprints 😄

eager pasture
ornate turtle
#

if i write a blueprint, then i want to get the git repository of the "app" usually

#

i guess we should probably support both actually 😄 not hard to do that

eager pasture
#

What about getting a source directory, and just calling asGit? Didn't you already add that?

ornate turtle
#

you can do this - but for a remote ref, the default depth is set to 1 (for efficiency, it's bad to clone the entire history just to get the code for the module)

#

so, the asGit result would just be a single commit, which is why we have so much hackery in the version module in the first place

#

i did consider just cloning everything to max depth for a directory, but it's a crap experience

eager pasture
ornate turtle
#

everything just slows right down

#

yup

eager pasture
#

I remember part of the motivation was to query relatively "small" metadata about the repo without having to download tons of data

ornate turtle
#

yup, that's pretty much it - you can very easily query a lot of the history, and for a lot of operations, it's going to be very quick, because those operations are cached because we've needed to do them to run the module code in the first place

eager pasture
#

Ironically that original issue is phrased as "git information about the context" which is not ambiguous even with blueprints

rotund cloud
# eager pasture What we're also curious about <@809456513298464798> is "option B" which would be...

I think I don't know enough about py/ts SDKs to say something here. When I'm reading func foo(source = dag.address(".").directory()) I feel that makes sense, but hard to say more, especially I don't know what were the previous limitations.
From Java SDK pov, to rename defaultPath to default (and add more types there) sounds ok. We already have some code that handles differently the default value (because of some nice things like enums...) So we should be able to extend it to handle directories, files, git, etc.

ornate turtle
eager pasture
#

dag.context().git()? 🙂

eager pasture
ornate turtle
#

yeahhhh? i mean the reason we didn't do this for directories is because that's bad for cache.

#

we need it to be in the schema somehow, so that we know when changes occur we can invalidate the cache

#

i remember a very long discussion about this before, i'm forgetting the finer points

#

personally, i think the contextual args is a neat solution (it also lets you override them!), i think it makes sense to extend it here

eager pasture
#

Yes that makes sense. I guess the reason it feels slightly sad in this particular case, is that all I want is to query a few basic fields about the current git context. Like "what's the current tag", "am I in a dirty commit". And although the default arg system is powerful and cool... it's complicated

#

Meahwhile a github actions workflow can go if [ $$GITHUB_BRANCH ] or whatever

ornate turtle
#

"am in a dirty commit" hell, yeah, i know, i want that api so badly

#

i tried, it's such a mess

eager pasture
#

You know what I mean though, maybe dirty commit is particulary hard to implement

ornate turtle
#

yeah yeah 🙂

eager pasture
#

but it feels different from contextual directories because it's much less open-ended

#

feels more like dag.CurrentModule().Source()

#

But maybe I'm just describing a different API, which could be smaller and layered on top of your PR as a convenience

ornate turtle
#

yeahhh

eager pasture
#

Since I wouldn't need the full power of GitRepository or even GitRef

#

just a bunch of scalars

ornate turtle
#

i think i'd like dag.CurrentModule().Git() which does the same as source (gets the source, not the blueprinted version)
and also defaultGit
maybe one day, we could think about dag.context()? i don't know how we'd do the cache but i have some ideas with theseus

#

e.g. we could configure the cache key post operation based on what you loaded

eager pasture
#

What does dag.CurrentModule() return today when running from a blueprint?

ornate turtle
#

it gives you the upstream source (the platform, not the app)

eager pasture
#

so the blueprint module

ornate turtle
#

yeahh

eager pasture
#

that makes sense to me

ornate turtle
#

sorry, not the "blueprinter?"

#

hehe

#

okay, i'll rework the pr:

  • add CurrentModule.Git
  • change to use defaultPath for now
eager pasture
ornate turtle
#

yeah! but if you want the downstream version, you use contextual defaultGit

#

just like you do with defaultPath

eager pasture
#

@ornate turtle if I want to know if I'm in a tag, or a branch, or neither, can I do that with CurrentModule.Git?

#

Just checking that we cover the basic CI use cases

ornate turtle
#

you can use Git.ref, which gives you the fully resolved ref. dagger -c 'git https://github.com/dagger/dagger.git | branch main | ref' gives you refs/heads/main

#

so refs/tag is tags, refs/head is branches

#

we can obviously add more helpers

#

i love the git type. honestly, just my favorite thing to work on.

#

i wanna add a way to author new commits (maybe using the patch api we recently added)
and then, being able to push would be awesome

eager pasture
#

My dream is to wire it to the CLI flow somehow, so that I can open a remote branch, go through a complete devloop completely stateless, and when I exit, all the changes get saved to a pre-arranged cow branch