Skip to content

[Bug] Error message isn't properly raised because abstract class is accessed during rendering of the error#5

Merged
matthewphsmith merged 2 commits into
masterfrom
fix-list-parse
Aug 23, 2019
Merged

[Bug] Error message isn't properly raised because abstract class is accessed during rendering of the error#5
matthewphsmith merged 2 commits into
masterfrom
fix-list-parse

Conversation

@matthewphsmith

Copy link
Copy Markdown
Collaborator

The exception fails to raise properly because an abstract class is used as the type comparison. This fixes that and adds two tests.

@matthewphsmith matthewphsmith added the bug Something isn't working label Aug 23, 2019

@chanadian chanadian left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good

@matthewphsmith matthewphsmith merged commit 25582ef into master Aug 23, 2019
max-hoffman pushed a commit to dolthub/flytekit that referenced this pull request May 11, 2021
[Bug] Error message isn't properly raised because abstract class is accessed during rendering of the error
ByronHsu pushed a commit to ByronHsu/flytekit that referenced this pull request May 29, 2023
* Pass flag verify to _upload_file & Fix some typos

* nit
eapolinario added a commit that referenced this pull request Jan 31, 2024
* Add output_entity_hint and support for serialising it

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Correct assertion

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Add output_entity_hint to task decorator

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Type hints

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Fix circular import of type hints

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Support iterable of output_entity_hints (#5)

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Fix circular import for type hints

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Fix tests

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Auto-format

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Add positive test case

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Add a test for disallowing entity hints on static tasks

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* raise instead of assert

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Rename output_entity_hints -> node_dependency_hints

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Update docstrings

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Auto-format

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Fix test_serialize_vscode

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Maybe fix the docs error

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Fix a newly added test from master

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

* Fix monodocs build

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>

---------

Signed-off-by: Thomas Newton <thomas.w.newton@gmail.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
noahjax referenced this pull request in dominodatalab/flytekit Mar 18, 2024
* remove cruft and add agent

* clean up updated agent, get it running locally

* switch to poetry to enable installing extras

* cleanup and black formatting

* dumb mypy change to pass ci

* ruff

* maybe fix test?

* fix logging, set log level

* black formatting and ruff fix

* update agent to create pipeline job

* cleanup agent, shuffle deps

* update Pipfile.lock

* black format

* black + mypy

* ruff

* update testt

* update test to stop testing DominoJobTask logic

* get dominoHost from a secret

* add os back

* [DOM-52824] Add pipeline interface args to pipeline config sent to job api (#6)

* add interfaces to pipelineConfig job api arg

* ruff

* update deps

* disabled test for now

* Re-enable test

* Appease the testing gods

* provide output paths

* supply inputMetadataPrefix to jobs api

* use same bucket for raw output

* Build container image with nonroot support

 Build with:

 > docker build -t train-flyte-domino-agent-service -f ./build/docker/Dockerfile .

 - With the previous container image

 ❯ docker run -i --rm --user 1000 --cap-drop all docker.io/library/train-flyte-domino-agent-service
Traceback (most recent call last):
  File "/usr/local/bin/pyflyte", line 5, in <module>
    from flytekit.clis.sdk_in_container.pyflyte import main
  File "/usr/local/lib/python3.9/site-packages/flytekit/__init__.py", line 208, in <module>
    from flytekit.core.base_sql_task import SQLTask
  File "/usr/local/lib/python3.9/site-packages/flytekit/core/base_sql_task.py", line 4, in <module>
    from flytekit.core.base_task import PythonTask, TaskMetadata
  File "/usr/local/lib/python3.9/site-packages/flytekit/core/base_task.py", line 31, in <module>
    from flytekit.core.context_manager import (
  File "/usr/local/lib/python3.9/site-packages/flytekit/core/context_manager.py", line 33, in <module>
    from flytekit.core.data_persistence import FileAccessProvider, default_local_file_access_provider
  File "/usr/local/lib/python3.9/site-packages/flytekit/core/data_persistence.py", line 513, in <module>
    data_config=DataConfig.auto(),
  File "/usr/local/lib/python3.9/site-packages/flytekit/configuration/__init__.py", line 606, in auto
    config_file = get_config_file(config_file)
  File "/usr/local/lib/python3.9/site-packages/flytekit/configuration/file.py", line 259, in get_config_file
    if current_location_config.exists():
  File "/usr/local/lib/python3.9/pathlib.py", line 1424, in exists
    self.stat()
  File "/usr/local/lib/python3.9/pathlib.py", line 1232, in stat
    return self._accessor.stat(self)
PermissionError: [Errno 13] Permission denied: 'flytekit.config'

 - With the updated container build

 ❯ docker run -i --rm --user 1000 --cap-drop all docker.io/library/train-flyte-domino-agent-service
 Starting up the server to expose the prometheus metrics...
 Starting the agent service...

* add datasetSnapshots to agent -> domino comms

* drop Pipfile in favor of poetry

* multi stage build

* add Pipfile back

* defer to nucleus for start pipeline job validation

* update readme

* add some debugging instructions

* black

* fix get domino host

* fix lint test

---------

Co-authored-by: integration-test integration-test <test-notifs+integration-test@dominodatalab.com>
Co-authored-by: ddl-ryan-connor <106334081+ddl-ryan-connor@users.noreply.github.com>
Co-authored-by: ddl-ebrown <ethan.brown@dominodatalab.com>
Co-authored-by: Ryan Connor <ryan.connor@dominodatalab.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants