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

updated black version to 20.8b1 #2769

Merged
merged 5 commits into from
Jan 4, 2021
Merged

Conversation

zyxue
Copy link
Contributor

@zyxue zyxue commented Dec 11, 2020

updated and reran black on python/seldon_core and python/tests

discussed on #2713 (comment)

Should black be run on all the pb2 files too like seldon_core/proto/prediction_pb2.py?

@seldondev
Copy link
Collaborator

Hi @zyxue. Thanks for your PR.

I'm waiting for a SeldonIO member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository.

python/Makefile Outdated
@@ -95,7 +95,7 @@ push_conda:
@echo "Alternatively use anaconda upload to publish on own channel"

setup_linter:
pip install black==19.3b0
pip install black==black 20.8b1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a typo?

Copy link
Contributor

@RafalSkolasinski RafalSkolasinski Dec 16, 2020

Choose a reason for hiding this comment

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

This should also be updated in requirements-dev.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the catch. updated

@RafalSkolasinski
Copy link
Contributor

Nice one!

Should black be run on all the pb2 files too like seldon_core/proto/prediction_pb2.py?

As these are auto-generated by protos compiler I don't think they should be covered by black.

@seldondev seldondev added size/S and removed size/XS labels Dec 16, 2020
@zyxue
Copy link
Contributor Author

zyxue commented Dec 16, 2020

Nice one!

Should black be run on all the pb2 files too like seldon_core/proto/prediction_pb2.py?

As these are auto-generated by protos compiler I don't think they should be covered by black.

I see. I used to format them for better readability, and also because black won't change the logic in anyway. But not blacking them sounds good too.

I've addressed your comments.

@RafalSkolasinski
Copy link
Contributor

/ok-to-test

@RafalSkolasinski
Copy link
Contributor

/test this

@RafalSkolasinski
Copy link
Contributor

/test integration

@seldondev
Copy link
Collaborator

Wed Dec 16 16:39:25 UTC 2020
The logs for [lint] [2] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/2.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=2

@seldondev
Copy link
Collaborator

Wed Dec 16 16:39:36 UTC 2020
The logs for [pr-build] [3] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/3.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=3

@seldondev
Copy link
Collaborator

Wed Dec 16 16:39:35 UTC 2020
The logs for [integration] [4] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/4.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=4

@seldondev
Copy link
Collaborator

Wed Dec 16 16:39:37 UTC 2020
The logs for [pr-build] [1] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/1.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=1

@RafalSkolasinski
Copy link
Contributor

/retest

@seldondev
Copy link
Collaborator

Wed Dec 16 17:35:25 UTC 2020
The logs for [integration] [5] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/5.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=5

@zyxue
Copy link
Contributor Author

zyxue commented Dec 16, 2020

when can I see the log of the failed integration test?

@RafalSkolasinski
Copy link
Contributor

there's a link in seldondev bot's message, they are available after the pipeline run

@zyxue
Copy link
Contributor Author

zyxue commented Dec 16, 2020

https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/5.log

=================================== FAILURES ===================================
__________________________ TestPrepack.test_tfserving __________________________
[gw1] linux -- Python 3.6.10 /usr/local/bin/python

self = <test_prepackaged_servers.TestPrepack object at 0x7f28bc4376a0>
namespace = 'test-tfserving'

    def test_tfserving(self, namespace):
        spec = "../../servers/tfserving/samples/mnist_rest.yaml"
        retry_run(f"kubectl apply -f {spec}  -n {namespace}")
        wait_for_status("tfserving", namespace)
>       wait_for_rollout("tfserving", namespace)

test_prepackaged_servers.py:97: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

sdep_name = 'tfserving', namespace = 'test-tfserving', attempts = 50, sleep = 5
expected_deployments = 1

    def wait_for_rollout(
        sdep_name, namespace, attempts=50, sleep=5, expected_deployments=1
    ):
        deployment_names = []
        for _ in range(attempts):
            deployment_names = get_deployment_names(sdep_name, namespace)
            deployments = len(deployment_names)
    
            if deployments == expected_deployments:
                break
            time.sleep(sleep)
    
        error_msg = (
            f"Expected {expected_deployments} deployment(s) but got {len(deployment_names)}"
        )
        assert len(deployment_names) == expected_deployments, error_msg
    
        for deployment_name in deployment_names:
            logging.info(f"Waiting for deployment {deployment_name}")
            for _ in range(attempts):
                ret = run(
                    f"kubectl rollout status -n {namespace} deploy/{deployment_name}",
                    shell=True,
                )
                if ret.returncode == 0:
                    logging.info(f"Successfully waited for deployment {deployment_name}")
                    break
                logging.warning(
                    f"Unsuccessful wait command but retrying for {deployment_name}"
                )
                time.sleep(sleep)
            assert (
                ret.returncode == 0
>           ), f"Wait for rollout of {deployment_name} failed: non-zero return code"
E           AssertionError: Wait for rollout of tfserving-default-0-mnist-model failed: non-zero return code

seldon_e2e_utils.py:158: AssertionError

this seems have nothing to do with the chage here...

@ukclivecox
Copy link
Contributor

Can you rebase as this failure is fixed

@zyxue
Copy link
Contributor Author

zyxue commented Dec 17, 2020

rebased. Will tests be triggered automatically?

@seldondev
Copy link
Collaborator

Thu Dec 17 16:55:27 UTC 2020
The logs for [pr-build] [6] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/6.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=6

@seldondev
Copy link
Collaborator

Thu Dec 17 16:55:32 UTC 2020
The logs for [lint] [7] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/7.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=7

@RafalSkolasinski
Copy link
Contributor

/test integration

@seldondev
Copy link
Collaborator

Thu Dec 17 17:04:59 UTC 2020
The logs for [integration] [8] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/8.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=8

@zyxue zyxue changed the title updated black to black 20.8b1 in setup_linter updated black version to 20.8b1 Dec 17, 2020
@zyxue
Copy link
Contributor Author

zyxue commented Dec 21, 2020

wondering if there is anything else that needs update for this PR?

@RafalSkolasinski RafalSkolasinski self-requested a review January 4, 2021 11:53
Copy link
Contributor

@RafalSkolasinski RafalSkolasinski left a comment

Choose a reason for hiding this comment

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

Fantastic job!
/lgtm
/approve

@RafalSkolasinski
Copy link
Contributor

/approve

@RafalSkolasinski
Copy link
Contributor

/lgtm

@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RafalSkolasinski

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@seldondev
Copy link
Collaborator

Mon Jan 4 11:56:48 UTC 2021
The logs for [pr-build] [9] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/9.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=9

@seldondev
Copy link
Collaborator

Mon Jan 4 11:56:50 UTC 2021
The logs for [lint] [10] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2769/10.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2769 --build=10

@seldondev seldondev merged commit f89aa40 into SeldonIO:master Jan 4, 2021
@zyxue zyxue deleted the part-0-black branch February 5, 2021 16:01
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.

4 participants