Unpin (almost) all the things!

[h-vetinari]

I saw that this feedstock is pinning a lot of packages that keep getting migrated, which makes arcticdb live in a parallel universe of outdated packages, rather than with the rest of the ecosystem living in the world (modulo migrations) defined by https://github.com/conda-forge/conda-forge-pinning-feedstock.

Further comments below.

[conda-forge-webservices]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

[h-vetinari]

Comments about individual pins that got removed

[h-vetinari]

I saw #5, but the libevent migration has been closed for 4 months; whatever external consumers of conda-forge packages must eventually move with the times.

[jjerphan]

We know, we are currently trying to have them update their base environment to use the latest versions of the core dependencies such as libevent's.

[h-vetinari]

Due to this cap, #57 has zero effect

[jjerphan]

libprotobuf < 4 had to be used for consistency with other constraints than the ones of conda-forge. I'll see if I can relax it now.

[h-vetinari]

krb5 is a crucial package underlying lots of fundamental packages; if you pin it (for longer than absolutely necessary; assuming anything breaks), you doom yourself.

[qc00]

ArcticDB does not use Kerberos directly. It's a transitive dependency of OpenSSL.

Would it work if we remove this explicit dependency?

[jjerphan]

We do not need to include transitive dependency, I think this one was added to work around a bug. Let's try remove to remove it now.

[jjerphan]

cyrus-sasl =2.1.27 might also be removable as a pin.

[h-vetinari]

OK cool. There's a bunch more things that the link check points out as not-actually-linked to. Those too would be candidates for removal, which would also reduce the "surface" of migrations you're exposed to.

Dependencies have a cost (not just in conda-forge BTW), so you shouldn't declare them (and then pay the price for having them) if you don't need them. ;-)

[jjerphan]

Dependencies have a cost (not just in conda-forge BTW), so you shouldn't declare them (and then pay the price for having them) if you don't need them. ;-)

Well, you are preaching to the choir; but some decisions makers chose back then to make this choice due to other constraints.

[h-vetinari]

Another hiccough in the pinning repo, fixed by conda-forge/conda-forge-pinning-feedstock#4928

[jjerphan]

I would be in favor to also unpin, but the newer versions of LMDB have slight changes of behavior compared to this one.

[h-vetinari]

In conda-forge/conda-forge-pinning-feedstock#4930 you were saying just now that the difference between 0.9.x and 0.9.x shouldn't matter?

[jjerphan]

Updating to the latest version of ArcticDB makes 1 test for ArcticDB fail, see: man-group/ArcticDB#437

IMO we could upgrade and xfail this test, but some do not want to have changes of behavior.

[h-vetinari]

LMBD is less critical because it has no other dependencies itself, so if you need to keep this on a non-main version, this will have a limited blast radius.

[jjerphan]

This one is manageable, yes.

[h-vetinari]

I didn't add the fmt 10 migrator here yet, because I don't know why this cap is there. In any case, the pinning is still at 9; but we should eventually be able to follow suit.

[jjerphan]

ArcticDB is currently incompatible with fmt 10, and I agree that we need it to.

[h-vetinari]

If keeping the lower bound for aws-crt-cpp is of importance to you, then you can still keep picking up the pinning by having an extra line (without any bounds or pins, so that the bot will pick it up).

Otherwise, since the pinning has moved beyond this lower bound already, you could just as well do:

Suggested change

	# Resolves https://github.com/man-group/ArcticDB/issues/465
	- aws-crt-cpp >=0.19.8
	# another line to pick up pinning
	- aws-crt-cpp
	- aws-crt-cpp

[jjerphan]

If keeping the lower bound for aws-crt-cpp is of importance to you, then you can still keep picking up the pinning by having an extra line (without any bounds or pins, so that the bot will pick it up).

That's a good thing to know. Is this documented somewhere?

With aws-crt-cpp<0.19.8, one of the AWS libraries was bugged; if the global pinning moved, we can remove the pin (I'll check that).

[jjerphan]

Hi @h-vetinari, thank you for starting this discussion!

I am entirely in favor of most (if not all) suggestions you made, and I also would like to make ArcticDB installable in as many environments as possible.

For now, developers of ArcticDB are facing several constraints which explain part of this feedstock's state:

• some consumers of ArcticDB use environments with some old versions of packages, most notably libevent 2.1.10. We are currently trying to see if they can update their setup for the best of everyone.
• bugs and segfaults were (and might still be) present for specific libraries. Pins on given version was a short term remedy, but we should spend time fixing the root cause of those problems. This has not been prioritized yet.
• ArcticDB's code-base is incompatible with the newest versions of some dependencies. This has not been prioritized yet.

I think we will first integrate such changes progressively via the rc branch for a smoother transition.

[jjerphan]

We know, we are currently trying to have them update their base environment to use the latest versions of the core dependencies such as libevent's.

[jjerphan]

ArcticDB is currently incompatible with fmt 10, and I agree that we need it to.

[jjerphan]

If keeping the lower bound for aws-crt-cpp is of importance to you, then you can still keep picking up the pinning by having an extra line (without any bounds or pins, so that the bot will pick it up).

That's a good thing to know. Is this documented somewhere?

With aws-crt-cpp<0.19.8, one of the AWS libraries was bugged; if the global pinning moved, we can remove the pin (I'll check that).

[jjerphan]

I would be in favor to also unpin, but the newer versions of LMDB have slight changes of behavior compared to this one.

[jjerphan]

libprotobuf < 4 had to be used for consistency with other constraints than the ones of conda-forge. I'll see if I can relax it now.

[h-vetinari]

Thanks for the quick reply!

For now, developers of ArcticDB are facing several constraints which explain part of this feedstock's state:

• some consumers of ArcticDB use environments with some old versions of packages, most notably libevent 2.1.10. We are currently trying to see if they can update their setup for the best of everyone.

They need to move on, or rather: the feedstock needs to. I don't know how the distribution between "some consumers" and "who pays articdb developers" is, but aside from extreme cases, we cannot keep conda-forge held back by some opaque proprietary set of packages. As always in FOSS, the easiest is to open-source things and benefit from the same process as everyone, but barring that: at least keep up with the changes.

I'll give you a concrete example why this affects conda-forge adversely: due to the libevent 2.10 builds, the aws-sdk-cpp migration stalled (see here), because the bot cannot find a legal combination of packages to satisfy both the new aws-sdk-cpp and old libevent. That means:

• the migration is stuck (for all of conda-forge; we could ignore arcticdb of course, but the situation gets more complicated once other packages start depending on that)
• it now needs manual debugging from a maintainer (likely someone from cf/core), rather than just passing through the normal automation
• if every feedstock did this, conda-forge would grind to a halt

So while bugs and incompatibilities are of course a valid reason for temporary pins & caps, they need to be removed with urgency, not "in a few months, when we get around to drawing down some tech debt".

• bugs and segfault were (and might still be) present for specific libraries. Pins on given version was a short term remedy, but we should spend time fixing the root cause of those problems. This has not been prioritized yet.
• ArcticDB's code-base is incompatible with the newest versions of some dependencies. This has not been prioritized yet.

I think it would be highly worthwhile to integrate whatever test suite articdb has into the feedstock recipe (rather than just import arcticdb). That way, you can ensure that things keep working even if we unpin something. For example (aside from py38 on osx for reasons not yet debugged), the builds here are looking fine even with libprotobuf 4.

[jjerphan]

OK, so the state of this feedstock is actually harming conda-forge as a whole (I would not have thought that it could block conda-forge-wide migrations for instance).

We thought about extending the tests on the feedstock, but we have had a bunch of other constraints and bugs in the meantime.

To me, updating the feedstock of ArcticDB now becomes a priority.

I would propose performing the changes as follow in dedicated PRs:

• remove the pin on libevent 2.1.10 and on msgpack 6: this would make the migration doable
• remove the pin on all other dependencies
• add new tests to the recipe: this would add 40 minutes to the CI run, making iterations last approximately 2 hours and 10 minutes
• remove the pin on libprotobuf

What do you think?

[h-vetinari]

OK, so the state of this feedstock is actually harming conda-forge as a whole (I would not have thought that it could block conda-forge-wide migrations for instance).

We thought about extending the tests on the feedstock, but we have had a bunch of other constraints and bugs in the meantime.

To me, updating the feedstock of ArcticDB now becomes a priority.

I appreciate it, thank you. To be 100% clear (even if it's not in my immediate interest to temper this motivation): it's not the end of the world, and it won't be the first nor the last time that this happens on a feedstock. I was trying to give you a concrete example where this stance causes extra friction & extra work, and we should strive to avoid that where possible (because c-f's resources are generally stretched even thinner than those of any one upstream library).

If we tackle the points that you mentioned at a reasonable pace, that would be great. You can ping me if something goes wrong or you have questions.

We thought about extending the tests on the feedstock, but we have had a bunch of other constraints and bugs in the meantime.

It can be as easy as running pytest tests/ in the test section. Of course, the first time probably needs a bit of iteration, and it's possible that some tests need to be skipped due to particularities of conda-forge (check out scipy or arrow for some pretty extensive examples), but it should not be a huge lift (unless the tests are completely broken already, but then: wouldn't it be better to know that earlier rather than later...?)

[jjerphan]

I was trying to give you a concrete example where this stance causes extra friction & extra work, and we should strive to avoid that where possible (because c-f's resources are generally stretched even thinner than those of any one upstream library).

Your explanations are clear and we know that the resources of conda-forge are pretty limited despite the extensive number of builds and feedstocks and the maintenance coordination and orchestration this necessitates.

You can ping me if something goes wrong or you have questions.

For now, the crux of the problem is to understand and inspect packages' dependency graph (in environment). Is is simple to now the dependencies of a package, but it is harder to:

• 1. know the common set of (transitive) dependencies of two packages (and inspect versions)
• 2. know the dependees of a package (and inspect versions)

In the case of this feedstock:

• for i. arcticdb and pyarrow depend (transitively) on libevent; for what I have observed if environment depends on pyarrow<12, then the environment is constraint to old versions of (transitive) dependencies including libevent==2.1.10.
• for ii. knowing the constraints of direct dependees of libevent would be useful. whoneeds repoqueries help, but this only allows partial inspections.

It can be as easy as running pytest tests/ in the test section.

We thought about this, but one of the reasons we choose not too (at least initially) was to have shorter CI checks. I do not have the exact numbers in mind, but for now they approximately take approximately 1h30 to complete. The python test suite is tested upstream on a few combinations for now and we are progressively extending it.

When I first mention testing, I was thinking of other tests than pytest's we could perform on the feedstock (e.g. check for proper cross-and protobuf-compilations).

---

Edit: it is worth knowing that nothing on conda-forge depends on arcticdb for now:

mamba repoquery whoneeds -c conda-forge arcticdb

conda-forge/noarch                                  14.2MB @   4.6MB/s  3.2s
conda-forge/linux-64                                34.4MB @   4.9MB/s  7.4s


No entries matching "arcticdb" found
arcticdb may not be installed. Try giving a channel with '-c,--channel' option for remote repoquery

[h-vetinari]

We thought about this, but one of the reasons we choose not too (at least initially) was to have shorter CI checks. I do not have the exact numbers in mind, but for now they approximately take approximately 1h30 to complete. The python test suite is tested upstream on a few combinations for now and we are progressively extending it.

As long as you're not in danger of running into the 6h timeout, I would strongly recommend to run as much of the test suite as possible, at least on the native (=x64) platforms. The feedstock doesn't get built nearly as often the upstream repo, so it shouldn't be such a big drain. Conda-forge does more or less complicated integration work - IMO it pays itself off many times over to just ensure everything as packaged works correctly before even publishing the artefacts.

[jjerphan]

I agree with you, we just would like to have all the test be run upstream first before having them on the repository because iterations are several hours long. It boils down to resource allocation discussions that we need to have within the project.

#61 has been opened to remove "minor" pins ; removing the pin on libevent 2.1.10 need further discussions.

Thank you for your patience.

[jjerphan]

@h-vetinari: probably you have seen but #64 and #63 have been merged, and the migration bot should be unlocked now. 🙂

Other pins are being removed progressively with:

• Remove pin on krb5, lmdbxx and cyrus-sasl #61
• [main] Sync with rc #70
• [libevent-2.1.10] Sync with rc #71

[jjerphan]

@h-vetinari: Since #88, #89, #61 and others PRs have been merged, do you have any objections to closing this issue?

[h-vetinari]

No, all good! Thanks for the effort!