Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Nov 29, 2023

Seems that we have a conflicting requirement coming for s3fs for some reason requiring older aibotocore. In order to investigate, we release a bit the requiremnt to allow to generate new constraints and to see where it came freom.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Seems that we have a conflicting requirement coming for s3fs for
some reason requiring older aibotocore. In order to investigate,
we release a bit the requiremnt to allow to generate new constraints
and to see where it came freom.
@potiuk
Copy link
Member Author

potiuk commented Nov 29, 2023

Not sure what the reason is - but I saw it happening yesterday in my "common.io" chicken-egg PR - however it started to happen now in main as well (likely because of new provider release). I need to investigate why we have s3fs forced to be lower version than most recent 2023.10.0 - for some reason it's being disallowed to be used or at least pip when resolving dependencies thinks 2023.9.2 should be used.

For now - temporary lowering down the limit we have in the image (which is mostly added to limit back-tracking of pip should hopefully help to resolve it (even if aibotocore will be downgraded in our constraints). But I will investigate what's the problem after.

@potiuk
Copy link
Member Author

potiuk commented Nov 29, 2023

Example failure here: https://github.com/apache/airflow/actions/runs/7028260044/job/19126641540

  ERROR: Cannot install None, aiobotocore>=2.7.0 and apache-airflow[aiobotocore,async,celery,cgroups,cncf-kubernetes,dask,deprecated-api,github-enterprise,google-auth,kerberos,ldap,leveldb,otel,pandas,password,rabbitmq,s3fs,saml,sentry,statsd,virtualenv]==2.8.0.dev0 because these package versions have conflicting dependencies.
  
  The conflict is caused by:
      The user requested aiobotocore>=2.7.0
      apache-airflow[aiobotocore,async,celery,cgroups,cncf-kubernetes,dask,deprecated-api,github-enterprise,google-auth,kerberos,ldap,leveldb,otel,pandas,password,rabbitmq,s3fs,saml,sentry,statsd,virtualenv] 2.8.0.dev0 depends on aiobotocore>=2.1.1; extra == "aiobotocore"
      s3fs 2023.9.2 depends on aiobotocore~=2.5.4
  
  To fix this you could try to:
  1. loosen the range of package versions you've specified
  2. remove package versions to allow pip attempt to solve the dependency conflict
  

@hussein-awala
Copy link
Member

I don't understand why it worked before; the s3fs requirement was always the same: https://github.com/fsspec/s3fs/blob/2023.9.2/requirements.txt#L1

Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@hussein-awala
Copy link
Member

hussein-awala commented Nov 29, 2023

Ah, it workerd with the last version 2023.10.0: https://github.com/fsspec/s3fs/blob/2023.10.0/requirements.txt#L1C1-L2C1

Maybe it's better to fix s3fs version? WDYT?

@hussein-awala
Copy link
Member

Let's wait for #35947 before merging.

@potiuk
Copy link
Member Author

potiuk commented Nov 29, 2023

Ah, it workerd with the last version 2023.10.0: https://github.com/fsspec/s3fs/blob/2023.10.0/requirements.txt#L1C1-L2C1

Maybe it's better to fix s3fs version? WDYT?

Yep. I know it worked - but for some reason (and I do not know yet why) it stopped. I tried to run my other PR #35890 which started to fail yesterday with s3fs >= 2023.10.0 and with recent incarnation of it - even with all the *fs >= 2023.10.0 and it did not work - it was causing infinite backtracking signalling that there is actually a conflict somewhere. It will need a bit of investigation to find out, so I prefer to relax it now and if it works, it will at least get the PRs green and allow to update other constraints.

I think there is something fishy with *fs dependencies (which I was afraid of from the beginning) have something to do with it - but what it is, I am not sure yet.

For now I will see if the relaxed dependencies will help.

@potiuk
Copy link
Member Author

potiuk commented Nov 29, 2023

Let's wait for #35947 before merging.

Yeah. Let's see - maybe it will behave differently - and maybe relaxing wont help either.

@hussein-awala
Copy link
Member

Ah I see!

which I was afraid of from the beginning

I had the same concerns too.

@potiuk
Copy link
Member Author

potiuk commented Nov 29, 2023

Nope. We seem to have bactracking with that one as well.

@potiuk potiuk closed this Nov 29, 2023
@potiuk
Copy link
Member Author

potiuk commented Nov 29, 2023

Closing in favour of #35952

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants