Skip to content

[WIP] Remove conda yaml #1135

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] Remove conda yaml #1135

wants to merge 1 commit into from

Conversation

ArtoLord
Copy link
Collaborator

@ArtoLord ArtoLord commented Oct 5, 2023

No description provided.

string python_version = 1; // 3.8, 3.9, ..., without patch
/* optional */ string pypi_index_url = 2; // Url of pypi index. If not set, using https://pypi.org/simple
repeated PythonPackage requirements = 3; // Requirements to install
repeated string local_modules_urls = 4; // Urls in storage to get local modules from
Copy link
Collaborator

@futujaos futujaos Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Возможно, с целью написания дебаг-логов на сервере, локальный модуль представлять не только как URL, но как пару (URL, original_path_on_client). Но не знаю, насколько это необходимо. В целом есть ощущение, что под локальный модуль хочется зарезервировать целый message, чтобы в случае чего это было расширяемо. Например, сейчас у нас по урлам лежат архивы, но это может поменяться, и нужна будет какая-то мета про то, что лежит в урле.

Copy link
Contributor

@vhaldemar vhaldemar Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also we could add some archive codec information here

Copy link
Collaborator Author

@ArtoLord ArtoLord Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will add message LocalModule here

@ArtoLord ArtoLord changed the title Remove conda yaml [WIP] Remove conda yaml Oct 5, 2023
string name = 1;
string yaml = 2;
repeated LocalModule local_modules = 3;
string python_version = 1; // 3.8, 3.9, ..., without patch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you repeating 1:1 PythonEnvSpec here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this is private api, and PythonEnvSpec is in public.

string name = 1; // Name of module
string url = 2; // Url in storage to get module from
string python_version = 1; // 3.8, 3.9, ..., without patch
/* optional */ string pypi_index_url = 2; // Url of pypi index. If not set, using https://pypi.org/simple
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you should add extra_index_url (repeated) and find_links (repeated) for future?
look at pip install --help

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be we just should pass requirements.txt file contents, it supports all features and options of piphttps://pip.pypa.io/en/stable/reference/requirements-file-format/, including extra_index_url and others. Then we can just install packages with pip, without conda.


message PythonPackage {
string name = 1;
string version = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also support things like package>=1.0,<2.0,!=1.5
and may be URL requirement (like Git)
so may be all requirements will be list of string, each string is requirement expression, like line in requirements.txt file
this could be non-secure though, we'll have to sanitize such expressions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants