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 2020/05/27 13:45:19 UTC

[GitHub] [airflow] Pymancer opened a new issue #9036: Task instance log_filepath doesn't include try_number

Pymancer opened a new issue #9036:
URL: https://github.com/apache/airflow/issues/9036


   Is it correct that ti.log_filepath doesn't include try_number?  How to get a valid path with it?
   https://github.com/apache/airflow/blob/8ac90b0c4fca8a89592f3939c76ecb922d93b3da/airflow/models/taskinstance.py#L385-L390 
   
   
   
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] alexbegg commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
alexbegg commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-677958620


   @mik-laj I agree, that would work too, just take it out of the default email template as well as the example of `html_content_template ` in the docs. It already has `log_url` anyways. It isn't useful especially if you use remote logging since you should get the log from the remote server anyways.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] dimberman commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
dimberman commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-812890750


   Hi @alexbegg @mik-laj,
   
   So what is the ultimate step here? Do we deprecate any log handlers or do we just make the log handler user the stable API internally?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] eladkal closed issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
eladkal closed issue #9036:
URL: https://github.com/apache/airflow/issues/9036


   


-- 
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 #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
potiuk closed issue #9036:
URL: https://github.com/apache/airflow/issues/9036


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj edited a comment on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-677945797


   > so it is showing the wrong path in emails and I would like to try and fix it and submit a PR.
   
   In my opinion, there is no universal method for determining the correct path to a file as there are TaskHandler which do not use files. For example: The StackdriverTaskHandler uses the ``"task_id"``, ``"dag_id"``,`` "execution_date"``, ``"try_number"`` labels which are added to each log entry. 
   
   I think, we should abandon the use of file paths. Third-party services can access the log using the stable API.
   https://airflow.readthedocs.io/en/latest/stable-rest-api-ref.html#operation/get_log
   
   You may try to use FileTaskHandler, but this is problematic and will not be compatible with all handlers.
   ```python
   from logging
   logging._handlers('task')._render_filename(ti, try_number).
   ```


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-653767220


   @potiuk We haven't deleted it yet. I wanted to make other changes before deleting this property.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] boring-cyborg[bot] commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-634671177


   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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-812891271


   @dimberman  I wanted to delete this attribute, but not everyone shares my opinion. See: https://github.com/apache/airflow/pull/9084#pullrequestreview-421803048
   
   CC: @kaxil 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-677945797


   > so it is showing the wrong path in emails and I would like to try and fix it and submit a PR.
   
   In my opinion, there is no universal method for determining the correct path to a file as there are TaskHandler which do not use files. For example: The StackdriverTaskHandler uses the ``"task_id"``, ``"dag_id"``,`` "execution_date"``, ``"try_number"`` labels which are added to each log. 
   
   We should abandon the use of file paths. Third-party services can access the log using the stable API.
   https://airflow.readthedocs.io/en/latest/stable-rest-api-ref.html#operation/get_log


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj edited a comment on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-677945797


   > so it is showing the wrong path in emails and I would like to try and fix it and submit a PR.
   
   In my opinion, there is no universal method for determining the correct path to a file as there are TaskHandler which do not use files. For example: The StackdriverTaskHandler uses the ``"task_id"``, ``"dag_id"``,`` "execution_date"``, ``"try_number"`` labels which are added to each log entry. 
   
   I think, we should abandon the use of file paths. Third-party services can access the log using the stable API.
   https://airflow.readthedocs.io/en/latest/stable-rest-api-ref.html#operation/get_log
   
   You may try to use FileTaskHandler, but this is problematic and will not be compatible with all handlers.
   ```python
   from logging
   logging._handlers('task')._render_filename(ti, try_number).
   ```
   
   Generally, logging is one area of the project that needs a little more attention as we are using too many hacks&tricks to make it work.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] alexbegg edited a comment on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
alexbegg edited a comment on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-677958620


   @mik-laj I agree, that would work too, just take it out of the default email template as well as the example of `html_content_template ` in the docs. It already has `log_url` anyways. The log path isn't useful especially if you use remote logging since you should get the log from the remote server anyways.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] alexbegg commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
alexbegg commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-677941227


   @mik-laj Is there an example of how to utilize `FileProcessorHandler` to get the log file path? The only thing I can find in the clode base to get a log file path that actually utilizes the correct `LOG_FILENAME_TEMPLATE` is:
   https://github.com/apache/airflow/blob/c35a01037ad4bb03d39a0319d37bc36b08ccf766/airflow/utils/helpers.py#L154 and https://github.com/apache/airflow/blob/c35a01037ad4bb03d39a0319d37bc36b08ccf766/airflow/utils/log/log_reader.py#L109
   
   ...
   
   Also, the deprecated `log_filepath` method is still used in https://github.com/apache/airflow/blob/010322692e6e3f0adc156f0beb81e267da0e97bb/airflow/models/taskinstance.py#L1612-L1619 so it is showing the wrong path in emails and I would like to try and fix it and submit a PR.
   
   Also since it can still be wrong in people's custom `html_content_template` as well because it has for a long time been shown in the docs page https://github.com/apache/airflow/blob/ba00db3d6cfbef2390c045edfeacf67e133513eb/docs/howto/email-config.rst#L50 I think it will be best to just fix `log_filepath` to utilize the new method to get the log path instead of deleting the property.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-636520376


   It seems that this property is no longer supported by Airflow and we should delete it. To generate a log path you must use FileProcessorHandler


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] eladkal commented on issue #9036: Task instance log_filepath doesn't include try_number

Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #9036:
URL: https://github.com/apache/airflow/issues/9036#issuecomment-931542110


   fixed in https://github.com/apache/airflow/pull/15217


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