Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pinning connexion==2.14.2 due to issue with latest release version3.0.0 #1354

Merged
merged 6 commits into from
Nov 6, 2023

Conversation

vatsrahul1001
Copy link
Contributor

@vatsrahul1001 vatsrahul1001 commented Nov 3, 2023

Pinning connexion==2.14.2 due to issue with latest release version3.0.0
more context:https://astronomer.slack.com/archives/C01UJJEN0P3/p1699003495317799

@ephraimbuddy
Copy link

Another option is to have --ignore-installed flag.

@@ -75,7 +75,7 @@ ENV PATH $PATH:$JAVA_HOME/bin::$HIVE_HOME/bin:$HADOOP_HOME/bin
COPY astronomer-providers /tmp/astronomer-providers
RUN python3 -m pip install --upgrade pip
# Ideally we should install using constraints file
RUN pip install --upgrade --force-reinstall --no-cache-dir /tmp/astronomer-providers[all]
RUN pip install --upgrade --no-cache-dir /tmp/astronomer-providers[all]
Copy link
Collaborator

@pankajkoti pankajkoti Nov 3, 2023

Choose a reason for hiding this comment

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

The problem with this would be that latest provider code from main would not be installed as it would already come installed with the runtime image and that would be non-latest.

Copy link
Collaborator

@pankajkoti pankajkoti Nov 3, 2023

Choose a reason for hiding this comment

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

may I know please what problem are we trying to solve? are we encountering some new issue that we did not before as this has been like this since quite a while, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pankajkoti correct, and we want these deployment with the main code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pankajkoti more context here

Copy link
Collaborator

Choose a reason for hiding this comment

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

okay, are we sure if it's that or the line below causing the reinstall of Airflow?
RUN pip install apache-airflow[slack]

How about we try changing this line instead to RUN pip install apache-airflow-providers-slack instead of removing the force reinstall?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per CI logs its getting installed because of reinstall of airflow

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6c84bbe) 98.57% compared to head (7b7478b) 98.57%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1354   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files          91       91           
  Lines        5458     5458           
=======================================
  Hits         5380     5380           
  Misses         78       78           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vatsrahul1001 vatsrahul1001 changed the title Removing force install of Astronomer Providers Pinning connexion==2.14.2 due to issue with latest release version3.0.0 Nov 6, 2023
@vatsrahul1001 vatsrahul1001 merged commit d745670 into main Nov 6, 2023
15 checks passed
@vatsrahul1001 vatsrahul1001 deleted the remove-force-install branch November 6, 2023 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants