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 2023/01/09 07:55:21 UTC

[GitHub] [airflow] farhan0syakir opened a new pull request, #28799: introduce dag processor job fix #27140

farhan0syakir opened a new pull request, #28799:
URL: https://github.com/apache/airflow/pull/28799

   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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 pull request #28799: introduce dag processor job fix #27140

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #28799:
URL: https://github.com/apache/airflow/pull/28799#issuecomment-1435984665

   Sorry for the delay::
   
   1. I think it would be good to maka a PR with this change.
   2. Yeah. rebase and add it. 
   
   @mhenc - also can yoy please let us know what you think about 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] farhan0syakir commented on pull request #28799: introduce dag processor job fix #27140

Posted by "farhan0syakir (via GitHub)" <gi...@apache.org>.
farhan0syakir commented on PR #28799:
URL: https://github.com/apache/airflow/pull/28799#issuecomment-1399285906

   Hi @potiuk 
   let me answer your question.
   1. I didn't. However, when I tried that, I got another question, is it possible to specify subdir in the current helm values.yaml?If not, probably need to raise another issue(?). Also for the liveness I use `airflow jobs check --hostname $(hostname)` which specify hostname so it will check it separately
   2.  I agree, I accidentally commited that in another branch, which I deleted now. In here the commit message is introduce dag processor job. Is it okay?


-- 
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 pull request #28799: introduce dag processor job fix #27140

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28799:
URL: https://github.com/apache/airflow/pull/28799#issuecomment-1387461523

   Looks cool @farhan0syakir. But I have two asks:
   
   1. Did you test it with more than one DagFileProcessor running on two different subdirs - each of them should be a potentially separate job and have separate liveness probe.
   
   2. Could you please add more description in the commit than `fixes: https://github.com/apache/airflow/issues/27140` ? The thing is that commits remain with us - while issues/PR history are kept in GitHub and we need to (as per ASF rules) keep all the context in Git, so we need to have more complete description o fhte contex and the fix in the commit.
   


-- 
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] mhenc commented on pull request #28799: introduce dag processor job fix #27140

Posted by "mhenc (via GitHub)" <gi...@apache.org>.
mhenc commented on PR #28799:
URL: https://github.com/apache/airflow/pull/28799#issuecomment-1438175707

   Thank you for looking int that.
   This change looks good for me. 
   TBH I haven't tested it with Helm (as we use other way of deploying workloads to K8s).
   


-- 
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 merged pull request #28799: introduce dag processor job fix #27140

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #28799:
URL: https://github.com/apache/airflow/pull/28799


-- 
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 pull request #28799: introduce dag processor job fix #27140

Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #28799:
URL: https://github.com/apache/airflow/pull/28799#issuecomment-1438180443

   Awesome work, congrats on your first merged pull request!
   


-- 
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] farhan0syakir commented on pull request #28799: introduce dag processor job fix #27140

Posted by GitBox <gi...@apache.org>.
farhan0syakir commented on PR #28799:
URL: https://github.com/apache/airflow/pull/28799#issuecomment-1375228693

   fix #27140


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