blag commented on a change in pull request #19099:
URL: https://github.com/apache/airflow/pull/19099#discussion_r733005856
##########
File path: airflow/providers/docker/example_dags/example_docker_copy_data.py
##########
@@ -75,7 +75,7 @@
"/bin/bash",
"-c",
"/bin/sleep 30; "
- "/bin/mv {{ params.source_location }}/" + f"{t_view.output}" + " {{
params.target_location }};"
+ "/bin/mv {{ params.source_location }}/" + str(t_view.output) + " {{
params.target_location }};"
Review comment:
This was a tough call.
The original string concatenation with an f-string just looks weird to me:
```python
"/bin/mv {{ params.source_location }}/" + f"{t_view.output}" + " {{
params.target_location }};"
```
Not even GitHub's syntax coloring understands what's going on there.
But on the other hand, making it a full f-string just looks weirder to me
since we need to render the double curly brackets:
```python
f"/bin/mv {{{{ params.source_location }}}}/{t_view_output} {{{{
params.target_location }}}};"
```
The preexisting f-string just served to implicitly coerce `t_view.output`
into a string, and I prefer an explicit coercion there. For me, the old school
string concatenation with `str(...)` reads the easiest:
```python
"/bin/mv {{ params.source_location }}/" + str(t_view.output) + " {{
params.target_location }};"
```
The same doesn't apply to the next line though, because we do have the
single quote and semicolon that we need to render in that f-string, so I left
that alone:
```python
"/bin/echo '{{ params.target_location }}/" + f"{t_view.output}';",
```
But I don't feel strongly about this, so if you do feel strongly, just reply
here and I'll change it to (or leave it) the way you like.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
