You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/10/10 16:13:07 UTC
[GitHub] [airflow] asaf400 opened a new issue #18874: DockerOperator xcom is broken
asaf400 opened a new issue #18874:
URL: https://github.com/apache/airflow/issues/18874
### Apache Airflow version
2.1.3
### Operating System
ubuntu 20.04
### Versions of Apache Airflow Providers
apache-airflow-providers-amazon==1.3.0
apache-airflow-providers-celery==1.0.1
apache-airflow-providers-databricks==1.0.1
apache-airflow-providers-docker==1.2.0
apache-airflow-providers-ftp==1.0.1
apache-airflow-providers-http==1.1.1
apache-airflow-providers-imap==1.0.1
apache-airflow-providers-oracle==1.1.0
apache-airflow-providers-postgres==1.0.2
apache-airflow-providers-presto==1.0.2
apache-airflow-providers-sftp==1.2.0
apache-airflow-providers-sqlite==1.0.2
apache-airflow-providers-ssh==1.3.0
apache-airflow-providers-tableau==1.0.0
### Deployment
Virtualenv installation
### Deployment details
_No response_
### What happened
When using DockerOperator with 'do_xcom_push' set as True,
Airflow documentation states that without xcom_all also True, the default False behavior is that airflow would push only the last line from docker into xcom return_value, but this statement is False
do_xcom_push just returns the entire \ chunked part of the entire docker output as a single 'merged' string,
all the lines, as far as I can tell..
See code lines:
https://github.com/apache/airflow/blob/10023fdd65fa78033e7125d3d8103b63c127056e/airflow/providers/docker/operators/docker.py#L258
lines is the generator that cli.attach returns earlier in the code, the generator comes from docker py, see here:
https://github.com/docker/docker-py/blob/7172269b067271911a9e643ebdcdca8318f2ded3/docker/api/client.py#L418
Following the api code for docker-py, and into the generator logic (socket read and stuff..)
we can conclude that the assumption that 'cli.attach' returns 'lines' is wrong, therefore do_xcom_push is braking the 'contract' set by the documentation of the parameter, it never actually returns the last line.
So it is probably better just to also use 'xcom_all' because that at least uses cli.logs which is a safer interface to use, until this issue is fixed, probably we need to create a matching issue in the docker-py repo, but maybe airflow should just handle the stream itself and split it into lines, returning the last line, see this example, modified from another issue on xcoms:
`from docker import APIClient
d = APIClient()
c = d.create_container(image='ubuntu:20.04', name='TEST', command="""bash -c "echo 'test' && echo 'test2'" """, host_config=d.create_host_config(auto_remove=False, network_mode='bridge'))
gen=d.attach(c['Id'], stderr=True, stdout=True, stream=True)
d.start(c['Id'])
print( [i for i in gen] )
print([i for i in d.logs(c['Id'], stream=True, stdout=True, stderr=True)])
d.remove_container(c['Id'])`
Thank you @nullhack
Related issues:
https://github.com/apache/airflow/issues/15952
https://github.com/apache/airflow/issues/9164
https://github.com/apache/airflow/issues/14809 - This is a real nasty issue, I'm also having
### What you expected to happen
_No response_
### How to reproduce
_No response_
### Anything else
_No response_
### Are you willing to submit PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939509376
Thanks for opening your first issue here! Be sure to follow the issue template!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #18874:
URL: https://github.com/apache/airflow/issues/18874
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939860833
Would you take a stab at fixing it ? Airflow has more than 1700 contributors - and you can be one of them. Seems that you already not only can test it but, also know the code how to fix it. Shall I assign it to you @asaf400 ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939575041
All the GitHub perm links I have published here are LATEST main/master branches of each repository,
therefore I actually think that I have done my due diligence enough to say the code doesn't seem to have changed from airflow 2.0.2 to airflow 2.1+, and that the issue would be just the same in new versions of airflow..
I myself am using 2.0.2 because that's what the MWAA over at AWS have enabled so far..
I have full respect for everyone officially involved in airflow development,
But even taking the most recent chunk of code from docker.py file in airflow's docker provider, you can see that this isn't fixed yet, and airflow component tests for this provider? I have no idea how they pass this feature test, it looks like the correct behavior is asserted, but in the 'real' world, it just doesn't compile:
https://github.com/apache/airflow/commit/cd3307ff2147b170dc3feb5999edf5c8eebed4ba#diff-942f75ae382d164822ff30e8138471be26102283dfcbf25e6f1c8e5795de0cb0R463
Code to replicate airflow's use of docker-py:
```
from docker import APIClient
d = APIClient()
c = d.create_container(image='ubuntu:20.04', name='TEST', command="""bash -c "echo 'test' && echo 'test2'" """, host_config=d.create_host_config(auto_remove=False, network_mode='bridge'))
lines = d.attach(container=c['Id'], stdout=True, stderr=True, stream=True)
d.start(c['Id'])
line = ''
res_lines = []
for line in lines:
if hasattr(line, 'decode'):
# Note that lines returned can also be byte sequences so we have to handle decode here
line = line.decode('utf-8')
line = line.strip()
res_lines.append(line)
print(line)
result = d.wait(c['Id'])
d.remove_container(c['Id'])
print(line)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-1021809379
No, let’s just fix the Docker log streaming issue here.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ferruzzi commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-1021647265
It looks like https://github.com/apache/airflow/pull/19030 is unrelated and, IMHO, undesirable since it is changing the default behavior of an existing Operator.
@eladkal @uranusjr - Do either of you feel that one needs to be finished and included here?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939509619
Documentation from docker-py
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
I have confirmed the fix I suggested works in airflow 2.0.2,
For my own use, I just overrode the 'DockerOperator' class, and it's '_run_image' function
I'll now start to work on a PR, branching out from latest main commit so that future versions of airflow would be fixed..
for any future refence for AWS MWAA users which are kind of stuck on airflow 2.0.2, copy the entire '_run_image' into a custom class with 'DockerOperator' as it's super:
1. remove this line:
`lines = self.cli.attach(container=self.container['Id'], stdout=True, stderr=True, stream=True)`
2. overwrite the entire code block that's similar to this, with this.. :) :
```python
try:
self.cli.start(self.container['Id'])
result = self.cli.wait(self.container['Id'])
lines = self.cli.logs(container=self.container['Id'], stdout=True, stderr=True, stream=True)
line = ''
res_lines = []
for line in lines:
```
and remove the `result = self.cli.wait(self.container['Id'])` line that comes some code lines later
As I have described before, the logs() function is presumably much more stable as it requests the entire container log from beginning to end via `/containers/{id}/logs` api, rather then the `/containers/{id}/attach` api, it all relies on docker's logging mechanisms, and shouldn't have any 'missing 1st line'
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939523442
This part of docker provider had gone through a number of changes and improvements and a number of bug have been fixed. What you are observing is very likley already fixed in latest `docker provider` - you will need to upgrade Airflow to 2.1 in order to use that provider however.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
I have confirmed the fix I suggested works in airflow 2.0.2,
For my own use, I just overrode the 'DockerOperator' class, and it's '_run_image' function
I'll now start to work on a PR, branching out from latest main commit so that future versions of airflow would be fixed..
for any future refence for AWS MWAA users which are kind of stuck on airflow 2.0.2, copy the entire '_run_image' into a custom class with 'DockerOperator' as it's super:
1. remove this line:
`lines = self.cli.attach(container=self.container['Id'], stdout=True, stderr=True, stream=True)`
2. overwrite the entire code block that's similar to this, with this.. :) :
```python
try:
self.cli.start(self.container['Id'])
result = self.cli.wait(self.container['Id'])
lines = self.cli.logs(container=self.container['Id'], stdout=True, stderr=True, stream=True)
line = ''
res_lines = []
for line in lines:
```
and remove the `result = self.cli.wait(self.container['Id'])` line that comes some code lines later
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
I have confirmed the fix I suggested works in airflow 2.0.2,
For my own use, I just overrode the 'DockerOperator' class, and it's '_run_image' function
I'll now start to work on a PR, branching out from latest main commit so that future versions of airflow would be fixed..
for any future refence for AWS MWAA users which are kind of stuck on airflow 2.0.2, copy the entire '_run_image' into a custom class with 'DockerOperator' as it's super:
1. remove this line:
`lines = self.cli.attach(container=self.container['Id'], stdout=True, stderr=True, stream=True)`
2. overwrite the entire code block that's similar to this, with this.. :) :
```python
try:
self.cli.start(self.container['Id'])
result = self.cli.wait(self.container['Id'])
lines = self.cli.logs(container=self.container['Id'], stdout=True, stderr=True, stream=True)
line = ''
res_lines = []
for line in lines:
```
and remove the `result = self.cli.wait(self.container['Id'])` line that comes some code lines later
As I have described before, the logs() function is much more stable as it requests the entire container log from beginning to end via `/containers/{id}/logs` api, rather then the `/containers/{id}/attach` api, it all relies on docker's logging mechanisms, and shouldn't have any 'missing 1st line'
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ferruzzi commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-1018738187
Looks like this is maybe abandoned? I can pick it up if @asaf400 isn't able to finish it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
I have confirmed the fix I suggested works in airflow 2.0.2,
For my own use, I just overrode the 'DockerOperator' class, and it's '_run_image' function
I'll now start to work on a PR, branching out from latest main commit so that future versions of airflow would be fixed..
for any future refence for AWS MWAA users which are kind of stuck on airflow 2.0.2, copy the entire '_run_image' into a custom class with 'DockerOperator' as it's super:
1. remove this line:
`lines = self.cli.attach(container=self.container['Id'], stdout=True, stderr=True, stream=True)`
2. overwrite the entire code block that's similar to this, with this.. :) :
```python
try:
self.cli.start(self.container['Id'])
result = self.cli.wait(self.container['Id'])
lines = self.cli.logs(container=self.container['Id'], stdout=True, stderr=True, stream=True)
line = ''
res_lines = []
for line in lines:
```
and remove the `result = self.cli.wait(self.container['Id'])` line that comes some code lines later
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939575041
All the GitHub perm links I have published here are LATEST main/master branches of each repository,
therefore I actually think that I have done my due diligence enough to say the code doesn't seem to have changed from airflow 2.0.2 to airflow 2.1+, and that the issue would be just the same in new versions of airflow..
I myself am using 2.0.2 because that's what the MWAA over at AWS have enabled so far..
I have full respect for everyone officially involved in airflow development,
But even taking the most recent chunk of code from docker.py file in airflow's docker provider, you can see that this isn't fixed yet, and airflow component tests for this provider? I have no idea how they pass this feature test, it looks like the correct behavior is asserted, but in the 'real' world, it just doesn't compile:
https://github.com/apache/airflow/commit/cd3307ff2147b170dc3feb5999edf5c8eebed4ba#diff-942f75ae382d164822ff30e8138471be26102283dfcbf25e6f1c8e5795de0cb0R463
Code to replicate airflow's use of docker-py:
```
from docker import APIClient
d = APIClient()
c = d.create_container(image='ubuntu:20.04', name='TEST', command="""bash -c "echo 'test' && echo 'test2'" """, host_config=d.create_host_config(auto_remove=False, network_mode='bridge'))
lines = d.attach(container=c['Id'], stdout=True, stderr=True, stream=True)
d.start(c['Id'])
line = ''
res_lines = []
for line in lines:
if hasattr(line, 'decode'):
# Note that lines returned can also be byte sequences so we have to handle decode here
line = line.decode('utf-8')
line = line.strip()
res_lines.append(line)
print(line)
result = d.wait(c['Id'])
d.remove_container(c['Id'])
print(line)
```
Edits:
- Code chunk formatting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939860833
Would you take a stab at fixin it ? Airflow has more than 1700 contributors - and you can be one of them. Seems that you already not only can test it but, also know the code how to fix it. Shall I assign it to you @asaf400 ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939575041
All the GitHub perm links I have published here are LATEST main/master branches of each repository,
therefore I actually think that I have done my due diligence enough to say the code doesn't seem to have changed from airflow 2.0.2 to airflow 2.1+, and that the issue would be just the same in new versions of airflow..
I myself am using 2.0.2 because that's what the MWAA over at AWS have enabled so far..
I have full respect for everyone officially involved in airflow development,
But even taking the most recent chunk of code from docker.py file in airflow's docker provider, you can see that this isn't fixed yet, and airflow component tests for this provider? I have no idea how they pass this feature test, it looks like the correct behavior is asserted, but in the 'real' world, it just doesn't compile:
https://github.com/apache/airflow/commit/cd3307ff2147b170dc3feb5999edf5c8eebed4ba#diff-942f75ae382d164822ff30e8138471be26102283dfcbf25e6f1c8e5795de0cb0R463
Code to replicate airflow's use of docker-py:
```
from docker import APIClient
d = APIClient()
c = d.create_container(image='ubuntu:20.04', name='TEST', command="""bash -c "echo 'test' && echo 'test2'" """, host_config=d.create_host_config(auto_remove=False, network_mode='bridge'))
lines = d.attach(container=c['Id'], stdout=True, stderr=True, stream=True)
d.start(c['Id'])
line = ''
res_lines = []
for line in lines:
if hasattr(line, 'decode'):
# Note that lines returned can also be byte sequences so we have to handle decode here
line = line.decode('utf-8')
line = line.strip()
res_lines.append(line)
result = d.wait(c['Id'])
d.remove_container(c['Id'])
print(line)
```
as per documentation and the pytest unittest, the output should be
`test2` right?
but it isn't, It's actually both
```test
test2```
Edits:
- Code chunk formatting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939575041
All the GitHub perm links I have published here are LATEST main/master branches of each repository,
therefore I actually think that I have done my due diligence enough to say the code doesn't seem to have changed from airflow 2.0.2 to airflow 2.1+, and that the issue would be just the same in new versions of airflow..
I myself am using 2.0.2 because that's what the MWAA over at AWS have enabled so far..
I have full respect for everyone officially involved in airflow development,
But even taking the most recent chunk of code from docker.py file in airflow's docker provider, you can see that this isn't fixed yet, and airflow component tests for this provider? I have no idea how they pass this feature test, it looks like the correct behavior is asserted, but in the 'real' world, it just doesn't compile:
https://github.com/apache/airflow/commit/cd3307ff2147b170dc3feb5999edf5c8eebed4ba#diff-942f75ae382d164822ff30e8138471be26102283dfcbf25e6f1c8e5795de0cb0R463
Code to replicate airflow's use of docker-py:
```python
from docker import APIClient
d = APIClient()
c = d.create_container(image='ubuntu:20.04', name='TEST', command="""bash -c "echo 'test' && echo 'test2'" """, host_config=d.create_host_config(auto_remove=False, network_mode='bridge'))
lines = d.attach(container=c['Id'], stdout=True, stderr=True, stream=True)
d.start(c['Id'])
line = ''
res_lines = []
for line in lines:
if hasattr(line, 'decode'):
# Note that lines returned can also be byte sequences so we have to handle decode here
line = line.decode('utf-8')
line = line.strip()
res_lines.append(line)
result = d.wait(c['Id'])
d.remove_container(c['Id'])
print(line) # issue is that: line != "test2"
```
as per documentation and the pytest unittest, the output should be
`test2` right?
but it isn't, It's actually both
```
test
test2
```
Edits:
- Code chunk formatting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941022822
@potiuk Yes, I can try and fix it, but it wouldn't be pretty,
Partly because the issue is also actually docker-py's fault..
I'll have to go through some of the old commits, but I think the resolution might be not using attach() at all, and just using logs() after container exits, and before container removal
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939523442
This part of docker provider had gone through a number of changes and improvements and a number of bug have been fixed. What you are observing is very likley already fixed in latest `docker provider` - you will need to upgrade Airflow to 2.1+ in order to use that provider however (or you can go straight to Airflow 2.2 which will be relesed shortly).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939523442
This part of docker provider had gone through a number of changes and improvements and a number of bug have been fixed. What you are observing is very likley already fixed in latest `docker provider` - you will need to upgrade Airflow to 2.1 in order to use that provider however (or you can go straight to Airflow 2.2 which will be relesed shortly).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939575041
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Nepherhotep commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
Nepherhotep commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-940493528
I can confirm the same issue with version 2.1.1 and 2.1.4 (those are the two only versions I tested).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
I have confirmed the fix I suggested works in airflow 2.0.2,
For my own use, I just overrode the 'DockerOperator' class, and it's '_run_image' function
I'll now start to work on a PR, branching out from latest main commit so that future versions of airflow would be fixed..
for any future refence for AWS MWAA users which are kind of stuck on airflow 2.0.2, copy the entire '_run_image' into a custom class with 'DockerOperator' as it's super:
1. remove this line:
`lines = self.cli.attach(container=self.container['Id'], stdout=True, stderr=True, stream=True)`
2. overwrite the entire code block that's similar to this, with this.. :) :
```python
try:
self.cli.start(self.container['Id'])
result = self.cli.wait(self.container['Id'])
lines = self.cli.logs(container=self.container['Id'], stdout=True, stderr=True, stream=True)
line = ''
res_lines = []
for line in lines:
```
and remove the `result = self.cli.wait(self.container['Id'])` line that comes some code lines later
As I have described before, the logs() function is presumably much more stable as it requests the entire container log from beginning to end via `/containers/{id}/logs` api, rather then the `/containers/{id}/attach` api, it all relies on docker's logging mechanisms, and shouldn't have any 'missing 1st line' issues
Edits: formatting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941022822
@potiuk Yes, I can try and fix it, but it wouldn't be pretty,
Partly because the issue is also actually docker-py's fault..
I mean, I'll have to go through some of the old commits, and see if my 'fix' doesn't revert some of the older commits, but I think the resolution might be not using attach() at all, and just using logs() after container exits, and before container removal.
in any case, I'll try to align the resolution with the pytest and documented expected behavior
Edits: rephrasing
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
I have confirmed the fix I suggested works in airflow 2.0.2,
For my own use, I just overrode the 'DockerOperator' class, and it's '_run_image' function
I'll now start to work on a PR, branching out from latest main commit so that future versions of airflow would be fixed..
for any future refence for AWS MWAA users which are kind of stuck on airflow 2.0.2 (as of now), copy the entire '_run_image' into a custom class with 'DockerOperator' as it's super:
1. remove this line:
`lines = self.cli.attach(container=self.container['Id'], stdout=True, stderr=True, stream=True)`
2. overwrite the entire code block that's similar to this, with this.. :) :
```python
try:
self.cli.start(self.container['Id'])
result = self.cli.wait(self.container['Id'])
lines = self.cli.logs(container=self.container['Id'], stdout=True, stderr=True, stream=True)
line = ''
res_lines = []
for line in lines:
```
and remove the `result = self.cli.wait(self.container['Id'])` line that comes some code lines later
As I have described before, the logs() function is presumably much more stable as it requests the entire container log from beginning to end via `/containers/{id}/logs` api, rather then the `/containers/{id}/attach` api, it all relies on docker's logging mechanisms, and shouldn't have any 'missing 1st line' issues
Edits: formatting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939575041
All the GitHub perm links I have published here are LATEST main/master branches of each repository,
therefore I actually think that I have done my due diligence enough to say the code doesn't seem to have changed from airflow 2.0.2 to airflow 2.1+, and that the issue would be just the same in new versions of airflow..
I myself am using 2.0.2 because that's what the MWAA over at AWS have enabled so far..
I have full respect for everyone officially involved in airflow development,
But even taking the most recent chunk of code from docker.py file in airflow's docker provider, you can see that this isn't fixed yet, and airflow component tests for this provider? I have no idea how they pass this feature test, it looks like the correct behavior is asserted, but in the 'real' world, it just doesn't compile:
https://github.com/apache/airflow/commit/cd3307ff2147b170dc3feb5999edf5c8eebed4ba#diff-942f75ae382d164822ff30e8138471be26102283dfcbf25e6f1c8e5795de0cb0R463
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939575041
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal closed issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
eladkal closed issue #18874:
URL: https://github.com/apache/airflow/issues/18874
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-1018812539
@ferruzzi assigned to you
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941237889
Would be cool!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-939860833
Would you take a stab at fixin it ? Airflow has more than 1700 contributors - seems that you already not only can test it but, also know the code how to fix it. Shall I assign it to you @asaf400 ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
I have confirmed the fix I suggested works in airflow 2.0.2,
For my own use, I just overrode the 'DockerOperator' class, and it's '_run_image' function
I'll now start to work on a PR, branching out from latest main commit so that future versions of airflow would be fixed..
for any future refence for AWS MWAA users which are kind of stuck on airflow 2.0.2 (as of time of writing), copy the entire '_run_image' into a custom class with 'DockerOperator' as it's super:
1. remove this line:
`lines = self.cli.attach(container=self.container['Id'], stdout=True, stderr=True, stream=True)`
2. overwrite the entire code block that's similar to this, with this.. :) :
```python
try:
self.cli.start(self.container['Id'])
result = self.cli.wait(self.container['Id'])
lines = self.cli.logs(container=self.container['Id'], stdout=True, stderr=True, stream=True)
line = ''
res_lines = []
for line in lines:
```
and remove the `result = self.cli.wait(self.container['Id'])` line that comes some code lines later
As I have described before, the logs() function is presumably much more stable as it requests the entire container log from beginning to end via `/containers/{id}/logs` api, rather then the `/containers/{id}/attach` api, it all relies on docker's logging mechanisms, and shouldn't have any 'missing 1st line' issues
Edits: formatting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] asaf400 edited a comment on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
asaf400 edited a comment on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941906832
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18874: DockerOperator xcom is broken
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18874:
URL: https://github.com/apache/airflow/issues/18874#issuecomment-941237889
Would be cool!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org