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

[sdk]: Replace TableLogger component in the SDK for better support with ipykernel>=6.x #1446

Closed
alembiewski opened this issue Oct 11, 2021 · 4 comments · Fixed by #1544
Closed

Comments

@alembiewski
Copy link
Member

The current version of table-logger package doesn't work with ipykernel >= 6.0.0 due to the recent changes in the latter, making writing string as bytes no longer supported.
We need to exclude table-logger dependency from the SDK and replace the TableLogger component used in *_job_watch.py for reporting job events with a more generic approach, making the watcher work with the newer ipykernel versions. Some suggestions include, but not limited to:

The original issue and more details can be found here: #1445

@alembiewski alembiewski changed the title [sdk]: Replace TableLogger component in the SDK for better support with ipykernel 6.x [sdk]: Replace TableLogger component in the SDK for better support with ipykernel>=6.x Oct 11, 2021
@alembiewski
Copy link
Member Author

/help
/good-first-issue

@google-oss-robot
Copy link

@alembiewski:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/help
/good-first-issue

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 kubernetes/test-infra repository.

@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale label Mar 2, 2022
@person142
Copy link
Contributor

To add some further activity to stave off stalebot - another reason to switch is that while kubeflow-training is licensed as Apache 2, table-logger is GPLv2.

@stale stale bot removed the lifecycle/stale label Mar 8, 2022
person142 added a commit to person142/training-operator that referenced this issue Mar 11, 2022
Closes kubeflow#1446.

This removes the dependency on `table-logger`. None of the various
alternatives mentioned in that issues were drop-in replacements, so
given the simplicity of the use case here the dependency is replaced
with a small class utilizing `str.format`. Note that the class was
written without referring to the `table-logger` source at all, which
would be a license violation.

I confirmed that you can now run the sample notebook

https://github.com/kubeflow/training-operator/blob/6523d8d2b7a27f1140a05971843cc4da7523694f/sdk/python/examples/kubeflow-tfjob-sdk.ipynb

in modern IPython.
google-oss-prow bot pushed a commit that referenced this issue Apr 20, 2022
Closes #1446.

This removes the dependency on `table-logger`. None of the various
alternatives mentioned in that issues were drop-in replacements, so
given the simplicity of the use case here the dependency is replaced
with a small class utilizing `str.format`. Note that the class was
written without referring to the `table-logger` source at all, which
would be a license violation.

I confirmed that you can now run the sample notebook

https://github.com/kubeflow/training-operator/blob/6523d8d2b7a27f1140a05971843cc4da7523694f/sdk/python/examples/kubeflow-tfjob-sdk.ipynb

in modern IPython.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants