Yes. I contemplated removing it completely and we could straight away fail an image that has it set and tell the user how to build the image and how to use the `--build` flag of docker-compose.
That was my second-best option. J. On Tue, Aug 29, 2023 at 2:49 PM Maciej Obuchowski < obuchowski.mac...@gmail.com> wrote: > I feel like straight up disallowing the feature, or disabling it in future > versions > and just strongly logging for now is the better option than failing after > 10 minutes. > > Generally I think introducing "feature" that would work locally and on dev > environment without noticeable problems - and failing on production is > wrong. > If I'm doing something wrong then either don't let me do that or just tell > me to not do it. > > For all the pros, they hold also for removing _PIP_ADDITIONAL_REQUIREMENTS > - so what's the rationale for still keeping that? > > wt., 29 sie 2023 o 14:30 Aritra Basu <aritrabasu1...@gmail.com> > napisał(a): > > > I haven't used this feature much yet, so not sure on the use case but the > > 10 minutes seems a bit arbitrary to me considering I do often test out > dags > > that take longer than 10 mins to run? Would it perhaps make more sense > that > > if you launch using the variable on any failure it throws an error saying > > you used this variable and that might cause this, retry without using > this > > and see if you still hit the issue? And perhaps send out warnings every X > > minutes/seconds saying you're using this feature which is not > recommended. > > I'm not sure how frequent warnings of that kind might discourage it's > usage > > in prod? > > > > -- > > Regards, > > Aritra Basu > > > > On Tue, Aug 29, 2023, 4:59 PM Andrey Anshin <andrey.ans...@taragol.is> > > wrote: > > > > > Airflow Trial Mode ON :D > > > > > > > It is aggressive, yes. I wonder what you think. Is it too aggressive? > > > > > > I think is not enough :D This one would be > > > > > > if [[ -n "${_PIP_ADDITIONAL_REQUIREMENTS=}" ]] ; then > > > echo "Longrid about best practices" > > > exit 42 > > > > > > In this case it is very easy to get a quick answer for an > > issue/discussion > > > like "My Airflow deployment does not start with exit code 42", rather > > than > > > "My Airflow deployment constantly restarted". > > > > > > But I'm not sure if it is applicable and user-friendly. > > > > > > ---- > > > Best Wishes > > > *Andrey Anshin* > > > > > > > > > > > > On Tue, 29 Aug 2023 at 15:01, Jarek Potiuk <ja...@potiuk.com> wrote: > > > > > > > Hello everyone, > > > > > > > > It happens more and more often (and seems that it is pretty > > > > widespread among "small" users of Airflow using Docker Compose and > > > official > > > > Airflow image that they are using _PIP_ADDITIONAL_REQUIREMENTS for > > > > production usage - even if building and hosting your modified image > is > > as > > > > easy as copy & pasting few lines to a Dockerfile and running `docker > > > build` > > > > following by `docker push. > > > > > > > > While I understand (and that's why we have it from the very > beginning) > > > why > > > > you would like to use it for local quick iteration and testing, using > > it > > > > for anything close to production has a lot of negative consequences > and > > > > leads to mysterious errors that take a lot of time for those users > who > > > are > > > > trying to investigating some dependency problems, slow start of > > > containers, > > > > race conditions and it makes them prone to security issues. > > > > > > > > I thought about whether we should remove the feature completely (it > is > > > > clearly marked as "do not use it for anything in production - so we > > > could). > > > > But I thought that a better solution could be to make the default > image > > > of > > > > ours simply unusable for production use if you use the flag. > > > > > > > > So I came up with this proposed PR: > > > > https://github.com/apache/airflow/pull/33879 > > > > > > > > It has a few protections against other side effects (such as > > accidentally > > > > downgrading or upgrading airflow by adding conflicting requirements, > > and > > > > verification if the dependencies pass `pip check`) - but the gist of > > it - > > > > the container will shut down automatically after 10 minutes if it has > > > been > > > > started with _PIP_ADDITIONAL_REQUIREMENTS (and will print > instructions > > > what > > > > to do - how to build your image - to avoid it). And we have much more > > > > comprehensive and detailed instructions on how to build your image > and > > > even > > > > how to integrate the "--build" docker-compose workflow to do the same > > > > following the way docker-compose was designed. > > > > > > > > Yes. It is super aggressive. Yes it will break a number of user's > > > workflows > > > > (especially when they are using 3rd-party charts that encourage using > > > > _PIP_ADDITIONAL_REQUIREMENTS) - part of the problem is that some > charts > > > out > > > > there are exposing this as a "production functionality" despite it > not > > > > being one. > > > > > > > > But IMHO - using the variable in production does more harm than good > > and > > > it > > > > has very easy alternatives, which users don't use because they choose > > > > shortcuts without realising the consequences - which is not the way > you > > > > should treat a production environment. > > > > > > > > Also restarting the container after 10 minutes is pretty consistent > > with > > > > the usage that _PIP_ADDITIONAL_REQUIRENENTS has been added for. > > > Restarting > > > > the container in a quick test/dependency iteration environment > > regularly > > > > could even be seen as a "feature" if you approach the "formal" status > > of > > > > this variable. > > > > > > > > Also this has a bit of a "sneaky" way of tricking people into > > > > "good practices" learning. In order to avoid the restart you really > > need > > > to > > > > .... build your own image ... So this has also an educational aspect > - > > > > people will have to learn about building the image anyway if they > will > > > want > > > > to learn how to "not build their images" and use something like the > > > > variable again and it will be even more complex than just adding the > > > > requirements to the custom image - so they will literally have to > learn > > > the > > > > "good practice". > > > > > > > > It is aggressive, yes. I wonder what you think. Is it too aggressive? > > > > Should we do it? > > > > > > > > Maybe there are really good reasons why it's impossible to use your > own > > > > custom images other than the extra step and annoyance of being able > to > > > > push your images to some - public (free) or private (internal) > > registry. > > > I > > > > can't imagine how someone deploying docker-compose or k8s does not > have > > > the > > > > way of having their own images available. Even if it means . that you > > > have > > > > to go through some kind of infra of yours you generally should do for > > > > production. > > > > > > > > Looking forward to your comments :D > > > > > > > > J. > > > > > > > > > >