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/07/18 17:52:57 UTC

[GitHub] [airflow] potiuk commented on issue #15952: DockerOperator tries to push bytes to XCom, fails to serialize

potiuk commented on issue #15952:
URL: https://github.com/apache/airflow/issues/15952#issuecomment-882094536


   I think this is somewhat ambiguous whether the APIs of docker return bytes or strings. Apparently it has been a problem before as  if you look few lines up, the conditional decode happens (and that's why we have line.encode("utf-8") that should stay there.
   
   ```
               for line in lines:
                   line = line.strip()
                   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')
                   res_lines.append(line)
   ```
   Not nice.
   
   I looked at the APIs https://docker-py.readthedocs.io/en/1.2.3/api/  and it is mentioned in both `attach` and `logs` that it will return either `str` or generator. It does not mention generator of what type (and we are using stream=Yes in attach so I guess we get the generator not str). So well, it's kinda not-specified whether the generator returns  bytes or strings and it's "OKeyish" to try to react to both cases.
   
   Interestingly, the documentation says that `attach` method is really a wrapper around logs() - and when you look closer at the method seems what we are trying to do is to retrieve the logs that we already retrieved (and stored in `res_lines` as array of `str`).  So it is likely that we actually receive again the same generator when we call logs after earlier calling "attach" with "stream=True".
   
   So I think the proper fix is actually: 
   
   ```
   ret = res_lines if self.xcom_all else line
   ```
   
   However I wonder maybe there was a good reason why the line was encoded to bytes before pushing to Xcom? I think if we fix it, we should make a major release of docker operator.
   


-- 
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