[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-137834 > Did another review. Starting to look nice and clear! Indeed. I like how this change now consolidates few places where we tried to communicate what should be considered as "public" in one dedicated place and I like the way how concise it is after all those corrections and feedback. I updated it now: * applied your corrections Bas (I actually made you co-author @BasPH if you do not mind, as I think there are enormous improvements in clarity from you here :) * i re-added the links to providers with just a "read more" - similarly as in other cases - I think this is consistent with all other parts and allows the reader to dig-deeper if they want. I hope we will be able to get it in soon :) -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1377861970 Hey @BasPH - just to repeat it - is your "change requested" still valid? Is the current approach good enough for you or do you think there is something stil not OK with teh approach I finally came up with after listening and getting feedback and applying multiple ideas on how to address the issue that has been already discussed in the devlist as needed ? I think we need to start openly communicate with our users on what they should expect as something they can rely on, And I think this is the first step tp do it. Anything else that needs to be done here do you thinkl? I would really love to merge it to move forward and seems it's been stalled on your review. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1372044348 @basph ? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1367581726 Ok. I brought it to the state that - I **hope** we can start discussing what is missing. I re-wrote the headers to reflect the current thinking of mine how our users could find this page useful, but also to make this page a really comprehensive and complete description of what the users might expect from Airflow as a "library". This is the current proposal for the "structure" of the page: https://user-images.githubusercontent.com/595491/210010256-6c8a1f35-ad24-4004-bebe-68afa5adfee5.png;> And I like that we can get rid of other pages that were "almost, but not quite entirely unlike what i wanted to achieve" (quoting my favourite writer). -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1367257938 Probably it can still be shortened and cleaned up a bit, but I think this way it is going to be precisely what I wanted to have - one place where we can tell people "If you want to do somethign with Airflow to extend it - here are all the possible ways you can do it when you want to make sure backwards compatibility is maintained". -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1367257038 Following the comments of @dstandish I just updated and restructured the proposal (again) - it will have some docs failures but I wanted to push it now if someone wants to look. I essentially merged all the different pages that were talking about the same with different focus - into one "Public Airflow Interface" where I described the intention of the page and described all the ways the public interfaces of Airflow can be used to extend Airflow functionality. It will be literally "all things Airflow considers public" with explaination why we create this single page and index to all the detailed pages and references where details can be found. I also added the general statement discussed with Daniel - that whatever is "_" or marked as meta:private in those classes, is not considered as "public". And left the general exclusion about Database, UI and "all other Python classes and methods that are not mentioned in this document. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1363215643 One other point (which very much concurs with what I described in my proposal about executor is actually something that @Taragolis noticed. https://github.com/apache/airflow/pull/28538#issuecomment-1363130466 Previously we could not really move Celery and Kubernetes executors to a provider, while with AiP-51 implemented in 2.6 we actually could. I am not sure if we would like to (CeleryKubernetesExecutor is a bit probllematic) - but in theory after AIP-51 gets implemented, both Celery and Kubernetes executor **could** be entirely moved into the providers. Whether we should do it or not is another story (and completely different discussion that we might be having), This is what I mean as truly decoupled and well defined interface @BasPH -> does it make sense. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1358275835 Any more comments to my explanation ? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1356830770 Rebased to apply static check fixes with isort. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1356180966 Rebased and applied the latest comments. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1355754952 Hey everyone - I spoke to @TohnJhomas and after we merged the big restructuring of his in #27235 we decided together to move all the changes for "customizing airflow" and "public-airlfow-interface" to "administration-and-deployment". This is mostly what it is all about - people writing DAGs are rarely concerned with changes that would involve breaking the API - those are more people who are installing and administering airlfow that are concerned about it, but I think we can still discuss what is the best place if others think otherwise. Looking forward to merging that one afte the big restructure of docs is merged. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1351052753 BTW. That was a good exercise (and learning) for GPT Chat after the reviews. I don't think it is good in writing docs like that - it's far to chatty (it's a chat bot eventually). Almost none of the original text from the GPT chat is left, and the number of omissions (which were added during the review) and mistakes it made was quite big. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1351030386 Hey @BasPH - explained reasoning why I addded those a bit longer explanation and context. I hope this is good for you as well. One of the cases which I want to address is to be able to explain people that if they do rely on some of those things being explained as "public interface" , even if they do, whenever they complain or raise questions about that I want to simply send them there - "Please read the docs, it's all explained there and you likely missed it". That saves enormous mental effort and time in disputes when you engage in such discussions (which are often) "but you did not explain it, I had the right to think it works this way". Also just to stress a bit that my "thinking" is not theorethical. It is very much based on practice of interacting with our users A LOT. I would encourage everyone to engage more in discussions we have in our community - or at least review them and follow. So far not many of us, maintainers are engaged, I think I understand quite well - and likely bettter than others - what our users are struggling with as well what is helpful for people. I am not sure if you realize, but over the last 6 months or so the status of Github Discussions of ours looks like that (I have - very consistently 11/15 "most helpful" answers last 30 days while the next person never had more than 1-2: https://user-images.githubusercontent.com/595491/207580648-449f0380-d37b-46d6-aeac-3996da820d5d.png;> And the raise in a number of discussions looks like that - so this is an extremely important medium where our users look for help: https://user-images.githubusercontent.com/595491/207581299-04ced62f-b4ae-47ed-aa9b-c11d8ee727ea.png;> -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1348978875 Next round of reviews completed. I've also added Trigger as it was missing. I think it's coming up nicely with everyone's contributions. Please take a look and see if there are other things I missing, maybe it is close to mergability BTW. The more review. the less GPT Chatbot's text remains:) . -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1347632747 (and docs building is fixed 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28300: Add Public Interface description to Airflow documentation
potiuk commented on PR #28300: URL: https://github.com/apache/airflow/pull/28300#issuecomment-1347617665 First round review passed - all comments applied @BasPH @o-nikolas -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
