Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lang): Support load syntax #559

Closed
VoVAllen opened this issue Jul 6, 2022 · 12 comments · Fixed by #808
Closed

feat(lang): Support load syntax #559

VoVAllen opened this issue Jul 6, 2022 · 12 comments · Fixed by #808

Comments

@VoVAllen
Copy link
Member

VoVAllen commented Jul 6, 2022

Description

Related: #557

Support load envd file from remote location, such as load('https://github.com/tensorchord/envd/raw/main/examples/mnist/')
or load('https://github.com/tensorchord/envd/raw/main/examples/mnist/build.envd')


Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

@aseaday
Copy link
Member

aseaday commented Jul 12, 2022

Should we have a proposal first?

@gaocegege
Copy link
Member

Yep, I think so. It is also related to #91

@aseaday
Copy link
Member

aseaday commented Jul 13, 2022

Maybe we should schedule a meeting for this?

@gaocegege
Copy link
Member

I am not sure if it is in high priority. There are few users interested in this now. WDYT

@VoVAllen
Copy link
Member Author

@aseaday I think #91 is the high priority now. load syntax is somehow builtin functionality of starlark to load function from other file. There're some default behaviors we can enable if needed.

@aseaday
Copy link
Member

aseaday commented Jul 13, 2022

@aseaday I think #91 is the high priority now. load syntax is somehow builtin functionality of starlark to load function from other file. There're some default behaviors we can enable if needed.

LGTM. I mentioned this because When I do menifest check in #553. The load syntax will introduce side effect like envd("http://").

@kemingy kemingy self-assigned this Aug 19, 2022
@VoVAllen
Copy link
Member Author

I'm actually thinking of rename config.jupyter to a new namespace, such as packages.jupyter, and try to use starlark implement it (like how we did in tensorboard and jupyter lab). Because config.jupyter contains both install and expose commands, which is not accurate under config namespace.

packages will contain lots of common-used softwares in the future and are open to user contributions, including tensorboard, mlflow, jupyter lab, openmmlab(which has complex dependencies), etc. . It can be a centralized recipes repository for packages that need extra cares (such as expose ports, system packages prerequisite and so on). And users can use those recipes easily in envd files directly or call load("packages") beforehand to explicitly load those packages.

WDYT? @kemingy @gaocegege @aseaday

@gaocegege
Copy link
Member

#559

The idea itself LGTM but I am not sure about the package name. Maybe jupyter and tensorboard should be in the preset, and the package name can be preset or something else. Packages is too general for me.

@aseaday
Copy link
Member

aseaday commented Aug 22, 2022

IMHO it is very important keyword and design. Even I don't have specfic ideas about how to load a llb function which other contributers write for a third-party packages like juypter or some not so famous package. We envd team can also be viewed as a special offical contributer.
As for packages and preset(s), I prefer preset but not as goog as a term could express the meaning which contains the potienal community contributed config module. maybe brew?

@kemingy
Copy link
Member

kemingy commented Aug 22, 2022

  1. The keyword load is already in starlark, I'm not sure if we want to override it.
  2. I prefer to define some internal functions and load them before config.envd. BTW, maybe we can introduce a preload directory (maybe .config/envd/preload/), so all the *.envd files under this folder will be evaluated before the users' build.envd file. Users can use the environment variable ENVD_PRELOAD_DIR to define their own directories.

@kemingy
Copy link
Member

kemingy commented Aug 22, 2022

  1. Plus, people can use use git to sync the files under ENVD_PRELOAD_DIR to make sure everybody in the team can share the same utilities.

@gaocegege
Copy link
Member

One possible scenario:

#557

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants