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 2022/12/27 19:10:09 UTC
[GitHub] [airflow] dharmendrakariya opened a new pull request, #28611: Fix scheduler-deployment.yaml
dharmendrakariya opened a new pull request, #28611:
URL: https://github.com/apache/airflow/pull/28611
While installing the chart with the git-sync, I was facing an issue, volume name was not available, This was just a typo!
--
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 #28611: Fix scheduler-deployment.yaml
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28611:
URL: https://github.com/apache/airflow/pull/28611#issuecomment-1366136357
cc: @jedcunningham - re our discussion - seems that even our chart thinks that dagPersistence and git sync is somehow at odds with each other.
--
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 #28611: Fix scheduler-deployment.yaml
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #28611:
URL: https://github.com/apache/airflow/pull/28611#issuecomment-1366119355
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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 #28611: Fix scheduler-deployment.yaml
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28611:
URL: https://github.com/apache/airflow/pull/28611#issuecomment-1366135959
Well. I think I have the same question - why would you want both persistence and git sync at the same time ? Was it a conscious choice or just accident (I think the problem would not be occuring if you had perstistence = False)
I think (this is my personal opinion though but there are different views) that dag Persistence = True and gitSync = True is a combination that makes very little sense - I can explain why. But before I do - I wonder - maybe you can explain why would you need it @dharmendrakariya ?
--
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] dharmendrakariya closed pull request #28611: Fix scheduler-deployment.yaml
Posted by GitBox <gi...@apache.org>.
dharmendrakariya closed pull request #28611: Fix scheduler-deployment.yaml
URL: https://github.com/apache/airflow/pull/28611
--
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 #28611: Fix scheduler-deployment.yaml
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28611:
URL: https://github.com/apache/airflow/pull/28611#issuecomment-1373567598
Glad it helped @dharmendrakariya !
cc: @jedcunningham -> do you think we should:
a) fix the issue with persistence + git sync in our chart
b) warn the users when they use persistence + git sync
c) disallow the "persistence + git sync" combination
?
As you might well guess, my personal preference is c) -> I **REALLY** think git sync + persistence is "never-needed" and "widely misleading" combo - that does not do what the users think it does and, in some cases (like above), does not even really work in our chart. I know you had some reservations for it in the past, but even after quite some time passed, I failed to see where this combo would be useful, and I would rather prevent or at the very least strongly warn our users when they use it. If we prevent this - we will have less of a matrix of possible cases to handle in the chart BTW.
Or maybe there is another way (for example fixing the problem found by @dharmendrakariya :)? WDYT @jedcunningham ?
--
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] dharmendrakariya commented on pull request #28611: Fix scheduler-deployment.yaml
Posted by GitBox <gi...@apache.org>.
dharmendrakariya commented on PR #28611:
URL: https://github.com/apache/airflow/pull/28611#issuecomment-1373560022
@potiuk closing this, you were right! Thanks bud!
--
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] dharmendrakariya commented on pull request #28611: Fix scheduler-deployment.yaml
Posted by GitBox <gi...@apache.org>.
dharmendrakariya commented on PR #28611:
URL: https://github.com/apache/airflow/pull/28611#issuecomment-1366155421
> Well. I think I have the same question - why would you want both persistence and git sync at the same time ? Was it a conscious choice or just accident (I think the problem would not be occuring if you had perstistence = False)
>
> I think (this is my personal opinion though but there are different views) that dag Persistence = True and gitSync = True is a combination that makes very little sense - I can explain why. But before I do - I wonder - maybe you can explain why would you need it @dharmendrakariya ?
Ohh, I didn't know that! I will try with Persistence = True, yeah true, you make sense! since all our dags are on version control and not on k8s, right?
--
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