-
Notifications
You must be signed in to change notification settings - Fork 0
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
New asset: download webcam images #177
Conversation
for book-keeping purposes: dagster-io/dagster#25490 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Several small remarks.
pipeline/__init__.py
Outdated
from .resources import JsonWebAssetIOManager, LamassuResource, PostGISGeoPandasIOManager | ||
from .resources.gdal import Ogr2OgrResource | ||
|
||
assets = load_assets_from_modules([sharing, radvis, gtfs, traffic_incidents]) | ||
assets = load_assets_from_modules([sharing, radvis, gtfs, traffic_incidents, webcams]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we make this multi-line, in order to have a git blame
-able formatting from now on?
assets = load_assets_from_modules([sharing, radvis, gtfs, traffic_incidents, webcams]) | |
assets = load_assets_from_modules([ | |
sharing, | |
radvis, | |
gtfs, | |
traffic_incidents, | |
webcams | |
]) |
asset, | ||
) | ||
|
||
SCRIPT_DIR = os.getenv('SCRIPT_DIR', './scripts/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should SCRIPT_DIR
be relative to $PWD
or to this file's path?
scripts/download_webcams.sh
Outdated
@@ -0,0 +1,5 @@ | |||
set -eo pipefail | |||
|
|||
echo Download webcams |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically, status/progress messages should be written to stderr
, so that the actual commands' outputs can be processed via stdout
.
echo Download webcams | |
1>&2 echo 'Downloading webcam images' |
This PR adds a
webcam_images
asset, which is materialized via a shell script wich transfers them via lftp.Dagster will only execute one materialization in parallel, so this asset may perform an initially long running transfer followed by (currently minutely executed) transfer jobs.
In case of a non-zero exit code of the shell script, a
RuntimeException
will be thrown which fails the materialization.stdout
andstderr
of the shell script are both captured and logged on the job's stderr.NOTE: this PR is still WIP. I.e. the script needs to be implemented (or mounted from another place). In case further env variables are intended to be propagated to the script, these would need to be handled as well.