Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/master' into wandb_feature_v2
Browse files Browse the repository at this point in the history
Signed-off-by: Thomas J. Fan <[email protected]>
  • Loading branch information
thomasjpfan committed May 16, 2024
2 parents 56584c9 + edab1e3 commit 7516fa3
Show file tree
Hide file tree
Showing 25 changed files with 528 additions and 175 deletions.
63 changes: 38 additions & 25 deletions .github/workflows/pythonbuild.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ jobs:
key: ${{ format('{0}-pip-{1}', runner.os, hashFiles('dev-requirements.in', 'requirements.in')) }}
- name: Install dependencies
run: |
make setup
pip uninstall -y pandas
pip freeze
pip install uv
make setup-global-uv
uv pip uninstall --system pandas
uv pip freeze
- name: Test with coverage
run: |
make unit_test_codecov
Expand Down Expand Up @@ -95,9 +96,10 @@ jobs:
key: ${{ format('{0}-pip-{1}', runner.os, hashFiles('dev-requirements.in', 'requirements.in')) }}
- name: Install dependencies
run: |
make setup
pip uninstall -y pandas
pip freeze
pip install uv
make setup-global-uv
uv pip uninstall --system pandas
uv pip freeze
- name: Run extras unit tests with coverage
# Skip this step if running on python 3.12 due to https://github.com/tensorflow/tensorflow/issues/62003
# and https://github.com/pytorch/pytorch/issues/110436
Expand Down Expand Up @@ -137,9 +139,10 @@ jobs:
key: ${{ format('{0}-pip-{1}', runner.os, hashFiles('dev-requirements.in', 'requirements.in')) }}
- name: Install dependencies
run: |
make setup
pip install --force-reinstall "${{ matrix.pandas }}"
pip freeze
pip install uv
make setup-global-uv
uv pip install --system --force-reinstall "${{ matrix.pandas }}"
uv pip freeze
- name: Test with coverage
run: |
make unit_test_codecov
Expand Down Expand Up @@ -172,7 +175,10 @@ jobs:
# Look to see if there is a cache hit for the corresponding requirements files
key: ${{ format('{0}-pip-{1}', runner.os, hashFiles('dev-requirements.in', 'requirements.in')) }}
- name: Install dependencies
run: make setup && pip freeze
run: |
pip install uv
make setup-global-uv
uv pip freeze
- name: Test with coverage
env:
FLYTEKIT_HYPOTHESIS_PROFILE: ci
Expand Down Expand Up @@ -207,7 +213,10 @@ jobs:
# Look to see if there is a cache hit for the corresponding requirements files
key: ${{ format('{0}-pip-{1}', runner.os, hashFiles('dev-requirements.in', 'requirements.in')) }}
- name: Install dependencies
run: make setup && pip freeze
run: |
pip install uv
make setup-global-uv
uv pip freeze
- name: Test with coverage
run: |
make test_serialization_codecov
Expand Down Expand Up @@ -244,7 +253,10 @@ jobs:
# Look to see if there is a cache hit for the corresponding requirements files
key: ${{ format('{0}-pip-{1}', runner.os, hashFiles('dev-requirements.in', 'requirements.in')) }}
- name: Install dependencies
run: make setup && pip freeze
run: |
pip install uv
make setup-global-uv
uv pip freeze
- name: Install FlyteCTL
uses: unionai-oss/flytectl-setup-action@master
- name: Setup Flyte Sandbox
Expand All @@ -263,6 +275,7 @@ jobs:
file: Dockerfile.dev
build-args: |
PYTHON_VERSION=${{ matrix.python-version }}
PSEUDO_VERSION=1.999.0dev0
push: true
tags: localhost:30000/flytekit:dev
cache-from: type=gha
Expand All @@ -272,7 +285,8 @@ jobs:
FLYTEKIT_IMAGE: localhost:30000/flytekit:dev
FLYTEKIT_CI: 1
PYTEST_OPTS: -n2
run: make integration_test_codecov
run: |
make integration_test_codecov
- name: Codecov
uses: codecov/[email protected]
with:
Expand All @@ -299,7 +313,8 @@ jobs:
- flytekit-data-fsspec
- flytekit-dbt
- flytekit-deck-standard
- flytekit-dolt
# TODO: remove dolt plugin - https://github.com/flyteorg/flyte/issues/5350
# flytekit-dolt
- flytekit-duckdb
- flytekit-envd
- flytekit-flyteinteractive
Expand Down Expand Up @@ -395,14 +410,15 @@ jobs:
key: ${{ format('{0}-pip-{1}', runner.os, hashFiles('dev-requirements.txt', format('plugins/{0}/requirements.txt', matrix.plugin-names ))) }}
- name: Install dependencies
run: |
pip install uv
# TODO: double-check if checking out all tags solves the issue
export SETUPTOOLS_SCM_PRETEND_VERSION="3.0.0"
make setup
make setup-global-uv
cd plugins/${{ matrix.plugin-names }}
pip install .
if [ -f dev-requirements.in ]; then pip install -r dev-requirements.in; fi
pip install -U $GITHUB_WORKSPACE
pip freeze
uv pip install --system .
if [ -f dev-requirements.in ]; then uv pip install --system -r dev-requirements.in; fi
uv pip install --system -U $GITHUB_WORKSPACE
uv pip freeze
- name: Test with coverage
run: |
cd plugins/${{ matrix.plugin-names }}
Expand Down Expand Up @@ -435,12 +451,9 @@ jobs:
- name: Install dependencies
run: |
python -m pip install --upgrade pip
make setup
pip freeze
pip install uv
make setup-global-uv
uv pip freeze
- name: Lint
run: |
make lint
- name: ShellCheck
uses: ludeeus/action-shellcheck@master
with:
ignore_paths: boilerplate
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ ARG DOCKER_IMAGE
# 3. Clean up the apt cache to reduce image size. Reference: https://gist.github.com/marvell/7c812736565928e602c4
# 4. Create a non-root user 'flytekit' and set appropriate permissions for directories.
RUN apt-get update && apt-get install build-essential -y \
&& pip install --no-cache-dir -U flytekit==$VERSION \
&& pip install uv \
&& uv pip install --system --no-cache-dir -U flytekit==$VERSION \
flytekitplugins-pod==$VERSION \
flytekitplugins-deck-standard==$VERSION \
scikit-learn \
Expand Down
23 changes: 15 additions & 8 deletions Dockerfile.dev
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ ENV FLYTE_SDK_RICH_TRACEBACKS 0
# Flytekit version of flytekit to be installed in the image
ARG PSEUDO_VERSION


# Note: Pod tasks should be exposed in the default image
# Note: Some packages will create config files under /home by default, so we need to make sure it's writable
# Note: There are use cases that require reading and writing files under /tmp, so we need to change its permissions.
Expand All @@ -26,15 +27,21 @@ ARG PSEUDO_VERSION
# 2. Install Flytekit and its plugins.
# 3. Clean up the apt cache to reduce image size. Reference: https://gist.github.com/marvell/7c812736565928e602c4
# 4. Create a non-root user 'flytekit' and set appropriate permissions for directories.
RUN apt-get update && apt-get install build-essential vim libmagic1 git -y
RUN pip install scikit-learn
RUN apt-get update && apt-get install build-essential vim libmagic1 git -y \
&& pip install uv

COPY . /flytekit
RUN SETUPTOOLS_SCM_PRETEND_VERSION_FOR_FLYTEKIT=$PSEUDO_VERSION pip install --no-cache-dir -U \
"git+https://github.com/flyteorg/flyte.git@master#subdirectory=flyteidl" \
-e /flytekit \
-e /flytekit/plugins/flytekit-k8s-pod \
-e /flytekit/plugins/flytekit-deck-standard \
-e /flytekit/plugins/flytekit-flyteinteractive \

# Use a future version of SETUPTOOLS_SCM_PRETEND_VERSION_FOR_FLYTEIDL such that uv resolution works.
RUN SETUPTOOLS_SCM_PRETEND_VERSION_FOR_FLYTEKIT=$PSEUDO_VERSION \
SETUPTOOLS_SCM_PRETEND_VERSION_FOR_FLYTEIDL=3.0.0dev0 \
uv pip install --system --no-cache-dir -U \
"git+https://github.com/flyteorg/flyte.git@master#subdirectory=flyteidl" \
-e /flytekit \
-e /flytekit/plugins/flytekit-k8s-pod \
-e /flytekit/plugins/flytekit-deck-standard \
-e /flytekit/plugins/flytekit-flyteinteractive \
scikit-learn \
&& apt-get clean autoclean \
&& apt-get autoremove --yes \
&& rm -rf /var/lib/{apt,dpkg,cache,log}/ \
Expand Down
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ update_boilerplate:
setup: install-piptools ## Install requirements
pip install -r dev-requirements.in

# Warning: this will install the requirements in your system python
.PHONY: setup-global-uv
setup-global-uv:
# Use "dev0" prefix to emulate version for dev environment
SETUPTOOLS_SCM_PRETEND_VERSION="1.999.0dev0" uv pip install --system -r dev-requirements.in

.PHONY: fmt
fmt:
pre-commit run ruff --all-files || true
Expand Down Expand Up @@ -105,7 +111,7 @@ requirements: doc-requirements.txt ${MOCK_FLYTE_REPO}/requirements.txt ## Compil
# TODO: Change this in the future to be all of flytekit
.PHONY: coverage
coverage:
coverage run -m pytest tests/flytekit/unit/core flytekit/types -m "not sandbox_test"
coverage run -m $(PYTEST) tests/flytekit/unit/core flytekit/types -m "not sandbox_test"
coverage report -m --include="flytekit/core/*,flytekit/types/*"

.PHONY: build-dev
Expand Down
4 changes: 2 additions & 2 deletions dev-requirements.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
-e file:.#egg=flytekit
git+https://github.com/flyteorg/flyte.git@master#subdirectory=flyteidl
-e file:.
flyteidl @ git+https://github.com/flyteorg/flyte.git@master#subdirectory=flyteidl

coverage[toml]
hypothesis
Expand Down
10 changes: 5 additions & 5 deletions flytekit/clis/sdk_in_container/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,11 @@ def get_entities_in_file(filename: pathlib.Path, should_delete: bool) -> Entitie
Returns a list of flyte workflow names and list of Flyte tasks in a file.
"""
flyte_ctx = context_manager.FlyteContextManager.current_context().new_builder()
if filename.is_relative_to("."):
module_name = str(filename.relative_to(".").with_suffix("")).replace(os.path.sep, ".")
if filename.is_relative_to(pathlib.Path.cwd()):
additional_path = str(pathlib.Path.cwd())
else:
module_name = str(filename.stem)
additional_path = str(filename.parent)
additional_path = _find_project_root(filename)
module_name = str(filename.relative_to(additional_path).with_suffix("")).replace(os.path.sep, ".")
with context_manager.FlyteContextManager.with_context(flyte_ctx):
with module_loader.add_sys_path(additional_path):
importlib.import_module(module_name)
Expand Down Expand Up @@ -532,7 +531,8 @@ def _run(*args, **kwargs):
raise click.UsageError(
f"Default for '{input_name}' is a query, which must be specified when running locally."
)
inputs[input_name] = processed_click_value
if processed_click_value is not None:
inputs[input_name] = processed_click_value

if not run_level_params.is_remote:
with FlyteContextManager.with_context(_update_flyte_context(run_level_params)):
Expand Down
11 changes: 9 additions & 2 deletions flytekit/core/data_persistence.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,11 @@ def raw_output_fs(self) -> fsspec.AbstractFileSystem:
return self._default_remote

def get_filesystem(
self, protocol: typing.Optional[str] = None, anonymous: bool = False, **kwargs
self,
protocol: typing.Optional[str] = None,
anonymous: bool = False,
path: typing.Optional[str] = None,
**kwargs,
) -> fsspec.AbstractFileSystem:
if not protocol:
return self._default_remote
Expand All @@ -195,6 +199,9 @@ def get_filesystem(
if anonymous:
kwargs["token"] = _ANON
return fsspec.filesystem(protocol, **kwargs) # type: ignore
elif protocol == "ftp":
kwargs.update(fsspec.implementations.ftp.FTPFileSystem._get_kwargs_from_urls(path))
return fsspec.filesystem(protocol, **kwargs)

storage_options = get_fsspec_storage_options(
protocol=protocol, anonymous=anonymous, data_config=self._data_config, **kwargs
Expand All @@ -204,7 +211,7 @@ def get_filesystem(

def get_filesystem_for_path(self, path: str = "", anonymous: bool = False, **kwargs) -> fsspec.AbstractFileSystem:
protocol = get_protocol(path)
return self.get_filesystem(protocol, anonymous=anonymous, **kwargs)
return self.get_filesystem(protocol, anonymous=anonymous, path=path, **kwargs)

@staticmethod
def is_remote(path: Union[str, os.PathLike]) -> bool:
Expand Down
8 changes: 6 additions & 2 deletions flytekit/core/promise.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,11 @@ def resolve_attr_path_in_promise(p: Promise) -> Promise:
break

# If the current value is a dataclass, resolve the dataclass with the remaining path
if type(curr_val.value) is _literals_models.Scalar and type(curr_val.value.value) is _struct.Struct:
if (
len(p.attr_path) > 0
and type(curr_val.value) is _literals_models.Scalar
and type(curr_val.value.value) is _struct.Struct
):
st = curr_val.value.value
new_st = resolve_attr_path_in_pb_struct(st, attr_path=p.attr_path[used:])
literal_type = TypeEngine.to_literal_type(type(new_st))
Expand Down Expand Up @@ -729,7 +733,7 @@ def binding_data_from_python_std(
lit = TypeEngine.to_literal(ctx, t_value, type(t_value), expected_literal_type)
return _literals_models.BindingData(scalar=lit.scalar)
else:
_, v_type = DictTransformer.get_dict_types(t_value_type)
_, v_type = DictTransformer.extract_types_or_metadata(t_value_type)
m = _literals_models.BindingDataMap(
bindings={
k: binding_data_from_python_std(
Expand Down
13 changes: 5 additions & 8 deletions flytekit/core/schedule.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,11 @@ def my_wf(kickoff_time: datetime): ...
kickoff_time_input_arg="kickoff_time")
"""
if cron_expression is None and schedule is None:
raise AssertionError("Either `cron_expression` or `schedule` should be specified.")

if cron_expression is not None and offset is not None:
raise AssertionError("Only `schedule` is supported when specifying `offset`.")

if cron_expression is not None:
CronSchedule._validate_expression(cron_expression)
if cron_expression:
raise AssertionError(
"cron_expression is deprecated and should not be used. Use `schedule` instead. "
"See the documentation for more information."
)

if schedule is not None:
CronSchedule._validate_schedule(schedule)
Expand Down
Loading

0 comments on commit 7516fa3

Please sign in to comment.