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

Reply via email to