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

removed usage of deprecated function for naming the pod in provider k8s pod.py #38638

Merged

Conversation

idantepper
Copy link
Contributor

(k8s/operator/pod.py): changed the function used to create pod name, before used a deprecated one and now using add_uniqu_suffix()

when executing this command
pytest tests/providers/cncf/kubernetes/operators/test_pod.py
received a warning that said we need to change the creation of pod name with a deprecated function


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

@boring-cyborg boring-cyborg bot added area:providers provider:cncf-kubernetes Kubernetes provider related issues labels Mar 30, 2024
@idantepper idantepper changed the title changed usage of deprecated function in provider k8s pod.py removed usage of deprecated function in provider k8s pod.py Mar 30, 2024
@idantepper idantepper changed the title removed usage of deprecated function in provider k8s pod.py removed usage of deprecated function for naming the pod in provider k8s pod.py Mar 30, 2024
Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

Yes this method is deprecated and we need to replace it by the new one, could you do the same thing for the other usages in the cncf.kubernetes provider?

@idantepper
Copy link
Contributor Author

idantepper commented Mar 30, 2024

i am goin over all of them right now, thanks for the respond

@idantepper
Copy link
Contributor Author

i changed a little bit more, i don't want this to be a huge PR, after doing this changes i will continue to change also the other ones

romsharon98

This comment was marked as duplicate.

romsharon98

This comment was marked as resolved.

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

There is just a missing one:

pod_id = add_pod_suffix(pod_name=pod_id, max_len=POD_NAME_MAX_LENGTH)

because we don't need to update the deprecated methods.

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 +1
Pending hussein's comment

…before used a deprecated one and now using add_uniqu_suffix()
idantepper@gmail.com added 4 commits April 4, 2024 00:50
…fore used a deprecated one and now using add_uniqe_id()
…nique_id to use add_unique_suffix before it used a deprecated one
…dd_unique_suffix before it used a deprecated one
@idantepper idantepper force-pushed the kubernetes-pod-operator-handle-warnings branch from 5041b0b to 7506940 Compare April 3, 2024 21:51
@idantepper
Copy link
Contributor Author

hello, after having some issues with my developing environment now everything is done
also added the change that hussein recommended.

thanks

@hussein-awala
Copy link
Member

@idantepper
Copy link
Contributor Author

i ran pre-commit and changed the order of the imports

@eladkal eladkal merged commit a19a9cb into apache:main Apr 4, 2024
49 checks passed
@idantepper idantepper deleted the kubernetes-pod-operator-handle-warnings branch April 4, 2024 21:36
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024
…8s pod.py (apache#38638)

* (k8s/operator/pod.py): changed the function used to create pod name, before used a deprecated one and now using add_uniqu_suffix()

* (k8s/operator/pod.py): changed the function used to create pod id, before used a deprecated one and now using add_uniqe_id()

* (k8s/operator/k8s_helper_functions.py): changed the function create_unique_id to use add_unique_suffix before it used a deprecated one

* changed order of imports

* (k8s/pod_generator.py): changed the function construct_pod() to use add_unique_suffix before it used a deprecated one

* changed imports order in pod_generator

---------

Co-authored-by: idantepper@gmail.com <idantepper@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:cncf-kubernetes Kubernetes provider related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants