Skip to content

Move build system to use pyproject.toml #3038

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Dec 14, 2022
Merged

Move build system to use pyproject.toml #3038

merged 16 commits into from
Dec 14, 2022

Conversation

akchinSTC
Copy link
Member

@akchinSTC akchinSTC commented Dec 1, 2022 •
edited
Loading

Signed-off-by: Alan Chin akchin@us.ibm.com

  • removed MANIFEST.in, setup.cfg, pytest.ini
  • use hatch engine instead of setuptools
  • moved labextension output directory due to hatch requirements (will not allow artifacts in dist dir to be placed in sdist)

TODO:

  • update release script
  • verify tests still function
  • address codeql issues
    - [ ] setup build hooks for lab extensions (stretch)

What changes were proposed in this pull request?

How was this pull request tested?

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@akchinSTC akchinSTC added component:build build and build related issues(dependencies and docker) status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. labels Dec 1, 2022
@akchinSTC akchinSTC marked this pull request as draft December 1, 2022 21:45
@akchinSTC akchinSTC requested a review from ajbozarth December 1, 2022 22:05
@kevin-bates
Copy link
Member

@akchinSTC - the primary changes from #3032 are not present here. I suspect this is because you had started this work prior to the merge of 3032 and since this PR removes pytest.ini and 3032 added a bunch of entries to it, the file's deletion takes precedence over content-reconciliation. As a result, the entries in 3032 (and any other recently merged PRs that might have modified files deleted in this PR) should have those changes added to their new locations in pyproject.toml.

@akchinSTC
Copy link
Member Author

@akchinSTC - the primary changes from #3032 are not present here. I suspect this is because you had started this work prior to the merge of 3032 and since this PR removes pytest.ini and 3032 added a bunch of entries to it, the file's deletion takes precedence over content-reconciliation. As a result, the entries in 3032 (and any other recently merged PRs that might have modified files deleted in this PR) should have those changes added to their new locations in pyproject.toml.

Thanks, I was totally rushing and missed that during the rebase

@akchinSTC akchinSTC force-pushed the pyproject branch 3 times, most recently from 018161d to 66f19a8 Compare December 2, 2022 16:16
@akchinSTC akchinSTC removed the status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. label Dec 2, 2022
@akchinSTC akchinSTC marked this pull request as ready for review December 2, 2022 16:17
kevin-bates
kevin-bates reviewed Dec 2, 2022
View reviewed changes
kevin-bates
kevin-bates reviewed Dec 2, 2022
View reviewed changes
kevin-bates
kevin-bates reviewed Dec 2, 2022
View reviewed changes
kevin-bates
kevin-bates reviewed Dec 2, 2022
View reviewed changes
kevin-bates
kevin-bates reviewed Dec 5, 2022
View reviewed changes
@ajbozarth
Copy link
Member

From a front end build perspective this look good, I also tested the dev on local lab functionality and it works with these changes, but this doc will need to be updated since the location of some of the lines that need to be edited has changed https://elyra.readthedocs.io/en/latest/developer_guide/development-workflow.html#developing-elyra-against-the-jupyterlab-source-repo

@akchinSTC akchinSTC mentioned this pull request Dec 6, 2022
Open
@ptitzler ptitzler added this to the 3.14.0 milestone Dec 6, 2022
@kevin-bates kevin-bates mentioned this pull request Dec 6, 2022
Merged
kevin-bates
kevin-bates reviewed Dec 6, 2022
View reviewed changes
kevin-bates
kevin-bates reviewed Dec 6, 2022
View reviewed changes
@akchinSTC akchinSTC force-pushed the pyproject branch 2 times, most recently from 99dfeb2 to 8b23716 Compare December 9, 2022 18:30
akchinSTC and others added 6 commits December 13, 2022 13:54
@akchinSTC
Move build system to use pyproject.toml
25ba4a2 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Re-add pytest ini configs from PR#3032
054cb45 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Update release script
79a5029 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Update dockerfile permissions
dcd57bd 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@kevin-bates @akchinSTC
Apply proper escaping for parenthesis in filterwarnings
aa3a2cc 
Signed-off-by: Kevin Bates <kbates4@gmail.com>
@akchinSTC
Bump black to 22.8.0 to support python 3.11 linting
d29cf2a 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Bump click dependency to v8+
0d35e2c 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Add test pytest_jupyter test dep
b0dda89 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Fix cap typo
5a54b4b 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Update dependency version floors
456c3ca 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Try codeql fixes
ca6f0d8 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Update to v2 codeql
c79538c 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Cap jupyterlab at <4
2f9d4ac 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Update release script to produce elyra-server
140ed70 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Add cap to jupyter-events for jsonschema
76042be
@akchinSTC
Introduce caps to avoid breaking changes
be6eecc 
Signed-off-by: Alan Chin <akchin@us.ibm.com>
@akchinSTC
Copy link
Member Author

@kevin-bates - if you're still lurking around.

Had to cap a few items in the last commit in this PR before merging

  • Cap on jupyter-core due to v5+ not supporting 3.7. (temp until we stop as well)
  • Cap on jupyter events due to kfp not supporting jsonschema v4
  • Cap on jupyter_server_terminals due to >0.4.0 causing issues with installation. See output below. I dont think we actually need this yet since it looks like its a dependency for jupyter_server v2. Unfortunately I think its getting pulled in during dependency resolution.

Will open issue to track if this is an ok workaround.

$ jupyter server extension list

Config dir: /Users/akchin/.jupyter

/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server_terminals/__init__.py:8: UserWarning: Could not import submodules
  warnings.warn("Could not import submodules")
Traceback (most recent call last):
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/manager.py", line 320, in add_extension
    extpkg = ExtensionPackage(name=extension_name, enabled=enabled)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/manager.py", line 166, in __init__
    super().__init__(*args, **kwargs)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/traitlets.py", line 1357, in __init__
    value = self._traits[key]._cross_validate(self, getattr(self, key))
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/traitlets.py", line 743, in _cross_validate
    value = obj._trait_validators[self.name](obj, proposal)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/traitlets.py", line 1229, in __call__
    return self.func(*args, **kwargs)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/manager.py", line 175, in _validate_name
    self._module, self._metadata = get_metadata(name)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/utils.py", line 58, in get_metadata
    return module, module._jupyter_server_extension_points()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server_terminals/__init__.py", line 15, in _jupyter_server_extension_points
    "app": TerminalsExtensionApp,
NameError: name 'TerminalsExtensionApp' is not defined

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/akchin/opt/anaconda3/envs/remove/bin/jupyter-server", line 8, in <module>
    sys.exit(main())
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_core/application.py", line 269, in launch_instance
    return super().launch_instance(argv=argv, **kwargs)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/config/application.py", line 992, in launch_instance
    app.start()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/serverapp.py", line 2814, in start
    self.start_app()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/serverapp.py", line 2710, in start_app
    super().start()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_core/application.py", line 258, in start
    self.subapp.start()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/serverextension.py", line 368, in start
    super().start()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_core/application.py", line 258, in start
    self.subapp.start()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/serverextension.py", line 342, in start
    self.list_server_extensions()
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/serverextension.py", line 323, in list_server_extensions
    config_dir, ext_manager = _get_extmanager_for_context(**option)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/serverextension.py", line 59, in _get_extmanager_for_context
    extension_manager = ExtensionManager(
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/config/configurable.py", line 86, in __init__
    super().__init__(**kwargs)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/traitlets.py", line 1364, in __init__
    self.notify_change(changes[key])
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/traitlets.py", line 1513, in notify_change
    return self._notify_observers(change)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/traitlets/traitlets.py", line 1560, in _notify_observers
    c(event)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/manager.py", line 253, in _config_manager_changed
    self._load_config_manager(change.new)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/manager.py", line 308, in _load_config_manager
    self.from_jpserver_extensions(jpserver_extensions)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/manager.py", line 313, in from_jpserver_extensions
    self.add_extension(name, enabled=enabled)
  File "/Users/akchin/opt/anaconda3/envs/remove/lib/python3.9/site-packages/jupyter_server/extension/manager.py", line 325, in add_extension
    if self.serverapp.reraise_server_extension_failures:
AttributeError: 'NoneType' object has no attribute 'reraise_server_extension_failures'
make: *** [check-install] Error 1

ptitzler
ptitzler approved these changes Dec 14, 2022
View reviewed changes
Copy link
Member

@ptitzler ptitzler left a comment

Choose a reason for hiding this comment

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

LGTM! confirmed successful builds using 3.7-3.11

@akchinSTC akchinSTC merged commit b9b9a98 into elyra-ai:main Dec 14, 2022
@kevin-bates
Copy link
Member

Thanks for moving forward with this PR in my absence.

  1. The jupyter_server_terminals issue looks like a server-related issue. I can try to look into this upon my return, not a big deal to cap that for now.
  2. The jupyter_events cap due (indirectly) to kfp is concerning. This will prevent others from using Elyra alongside event-related functionality (not a big deal right now) and we may very well want to use the event system ourselves as well. We should probably create an issue to revisit the jsonschema cap and how kfp v1 and jupyter_events can co-exist.
  3. The introduction of write_error() calling the mixin's write_error() strikes me as unnecessary - and I thought I left had left a comment to this affect, but either it was blown away by a forced push or I forgot to save it. Was the need for this triggered by a linting issue? The Mixin's write_error() implementation should resolve and prevent the need for the override.

@akchinSTC
Copy link
Member Author

akchinSTC commented Dec 14, 2022 •
edited
Loading

  1. The jupyter_server_terminals issue looks like a server-related issue. I can try to look into this upon my return, not a big deal to cap that for now.

@kevin-bates - Sorry for bothering you on vacation. Yeah this package is causing some headaches atm. pips resolver continues to pull in jupyter_server >2, thus pulling in this package causing issues. Capping the terminals pkg still doesnt resolve the issue, as a result terminals are not working in the latest 3.14 release. Need a way avoid pulling jupyter_server >2 completely. I have tried capping it but the resolver still pulls in jupyter_server > 2......then.....resolves and installs the capped version...leaving the problematic dependency in place.

@akchinSTC akchinSTC mentioned this pull request Jan 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevin-bates kevin-bates kevin-bates left review comments

@ptitzler ptitzler ptitzler approved these changes

@ajbozarth ajbozarth Awaiting requested review from ajbozarth

Assignees
No one assigned
Labels
component:build build and build related issues(dependencies and docker)
Projects
None yet
Milestone
3.14.0
Development

Successfully merging this pull request may close these issues.
4 participants
@akchinSTC @kevin-bates @ajbozarth @ptitzler