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

Drop use of walrus operator #993

Merged
merged 1 commit into from
Jan 9, 2024
Merged

Drop use of walrus operator #993

merged 1 commit into from
Jan 9, 2024

Conversation

freyes
Copy link
Contributor

@freyes freyes commented Dec 14, 2023

Description

The walrus operator (:=) was introduced in Python 3.8, the use of it in the 2.9 branch breaks python 3.6 compatibility which is the version used in Ubuntu 18.04 (Bionic).

https://docs.python.org/3/whatsnew/3.8.html#assignment-expressions

QA Steps

  1. Install python-libjuju in python 3.6
  2. python3 -c "import juju.utils"
virtualenv -p /usr/bin/python3.6 /tmp/myvenv
source /tmp/myvenv/bin/activate
python3 setup.py install
python3 -c "import juju.utils"

All CI tests need to pass.

Notes & Discussion

This regression was introduced by #975

freyes added a commit to freyes/zaza that referenced this pull request Dec 14, 2023
The python-libjuju 2.9.46 release introduced the use of the walrus
operator which is incompatible with python 3.6, more details at
juju/python-libjuju#993
@cderici
Copy link
Contributor

cderici commented Dec 14, 2023

If this is the only breaking point for 3.6, I won't be against removing this particular use of the walrus, just to unblock you.

However, it's important to note that we dropped support for python < 3.8 a long while ago, and will soon be moving to support 3.11-3.12 and 3.13. So you should expect more things to fail if you continue using python < 3.8.

freyes added a commit to freyes/zaza that referenced this pull request Dec 14, 2023
The python-libjuju 2.9.45 release introduced the use of the walrus
operator which is incompatible with python 3.6, more details at
juju/python-libjuju#993
juju/utils.py Outdated
# Secondly check: $XDG_DATA_HOME for ~/.local/share
elif xdg_data_home := os.environ.get('XDG_DATA_HOME'):
xdg_data_home = os.environ.get('XDG_DATA_HOME')
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to get this if the juju_data is available. Please put it in the else case down below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack. Addressed by f05162e

@freyes
Copy link
Contributor Author

freyes commented Dec 14, 2023

If this is the only breaking point for 3.6, I won't be against removing this particular use of the walrus, just to unblock you.

However, it's important to note that we dropped support for python < 3.8 a long while ago, and will soon be moving to support 3.11-3.12 and 3.13. So you should expect more things to fail if you continue using python < 3.8.

I understand the desire (and need) to drop support for old versions of python, although this is the 2.9 branch, where the expectation to support Ubuntu 18.04 sounds reasonable to me.

@freyes freyes requested a review from cderici January 2, 2024 21:57
Copy link
Contributor

@cderici cderici left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@cderici
Copy link
Contributor

cderici commented Jan 8, 2024

/merge

@cderici
Copy link
Contributor

cderici commented Jan 8, 2024

@freyes All the commits need to be signed to be able to land this, please rebase with git config --global commit.gpgSign true

@freyes
Copy link
Contributor Author

freyes commented Jan 9, 2024

@freyes All the commits need to be signed to be able to land this, please rebase with git config --global commit.gpgSign true

since I will need to do a "push --force", do you mind if I squash those two commits into a single one?, it would make history cleaner.

The walrus operator (`:=`) was introduced in Python 3.8, the use of it
in the `2.9` branch breaks python 3.6 compatibility which is the version
used in Ubuntu 18.04 (Bionic).

https://docs.python.org/3/whatsnew/3.8.html#assignment-expressions
Copy link
Contributor

@cderici cderici left a comment

Choose a reason for hiding this comment

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

since I will need to do a "push --force", do you mind if I squash those two commits into a single one?, it would make history cleaner.

I'd even prefer it that way, thanks a lot for taking care of this, this is good to go 👍

@cderici
Copy link
Contributor

cderici commented Jan 9, 2024

/merge

@jujubot jujubot merged commit df709e3 into juju:2.9 Jan 9, 2024
11 of 12 checks passed
@freyes freyes deleted the fix-syntax branch January 9, 2024 19:10
@cderici cderici mentioned this pull request Feb 8, 2024
jujubot added a commit that referenced this pull request Feb 13, 2024
#1023

## What's Changed
* Drop use of walrus operator by @freyes in #993
* No controller model access needed for connection with a non-admin user by @cderici in #1003
* Password resolution in connector by @cderici in #1002
* Remove dependency to juju-cli for controller_name by @cderici in #1009

#### Notes & Discussion

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

3 participants