-
Notifications
You must be signed in to change notification settings - Fork 356
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
refactor: combine common, cli, deploy into one python package [DET-4756] #2108
Conversation
a2e2bbd
to
3c9ebff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, nice work
], | ||
entry_points={"console_scripts": ["det-deploy = determined_deploy.__main__:main"]}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think that the det-deploy
executable belongs to the determined_deploy
package, and that the main repo should only contain det deploy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking of det deploy
as a separate chunk of work, for a later, separate PR. Then keeping det-deploy
in determined_deploy
defeats the whole purpose of merging it into the main package.
We can keep both det-deploy
and det deploy
in the main package to ease the migration, print a deprecation in det-deploy
, and then eventually remove it.
- reapply some work from determined-ai#2108 that was reverted in determined-ai#2153 and determined-ai#2156. - add more docs.
Tests were moved from ./cli to ./harness in determined-ai#2108 but the make target was not updated. Here we remove it so that the absence of the tests directory does not cause `make test` to fail.
* fix: remove test-cli from make test Tests were moved from ./cli to ./harness in #2108 but the make target was not updated. Here we remove it so that the absence of the tests directory does not cause `make test` to fail. * fix: remove test-common from make test Tests were moved from ./common to ./harness in #2018 but the make target was not updated. Here we remove it so that the absence of the tests directory does not cause `make test` to fail.
* fix: remove test-cli from make test Tests were moved from ./cli to ./harness in determined-ai#2108 but the make target was not updated. Here we remove it so that the absence of the tests directory does not cause `make test` to fail. * fix: remove test-common from make test Tests were moved from ./common to ./harness in determined-ai#2018 but the make target was not updated. Here we remove it so that the absence of the tests directory does not cause `make test` to fail.
Description
determined-common
,determined-cli
,determined-deploy
python packages are now DEPRECATED as they've been combined intodetermined
package.harness/determined/<OLD_PACKAGE_NAME>
determined_{common,deploy,cli}
todetermined.{common,deploy,cli}
Test Plan
Commentary (optional)
Checklist
docs/release-notes/
.See Release Note for details.