Skip to content

package installation in non-editable mode: pip install ./cli #550

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

Closed
wants to merge 25 commits into from

Conversation

VukW
Copy link
Contributor

@VukW VukW commented Feb 27, 2024

The goal is to make medperf client installable from pypi. This PR is the first part that allows installing package not from repository

A few changes were implemented:

  • Totally removed git branch check updates: as client installed in non-editable mode would not have any connections with source folder where it was installed from, it knows nothing about git branch.
  • During deployment, local server places cert.crt, cert.key and mock_tokens/tokens.json to ~/.medperf_config/.local_server/ so detached medperf client can use it. NB: this change breaks the behavior for existing developers (as path to local server cert is already defined in user config). You should update local config's certificate path to ~/.medperf_config/.local_server/cert.crt.
  • pip install -e ./cli replaced with pip install ./cli everywhere

@VukW VukW requested a review from a team as a code owner February 27, 2024 18:19
@VukW VukW had a problem deploying to testing-external-code February 27, 2024 18:19 — with GitHub Actions Failure
Copy link
Contributor

github-actions bot commented Feb 27, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@VukW VukW had a problem deploying to testing-external-code February 27, 2024 18:35 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 27, 2024 18:51 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 13:42 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 13:44 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 13:56 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 14:05 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 14:12 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 14:19 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 14:27 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 14:38 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 14:47 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 14:53 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 15:01 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 15:12 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 15:15 — with GitHub Actions Failure
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 15:24 — with GitHub Actions Failure
@@ -1,16 +1,13 @@
from ._version import __version__
from pathlib import Path

BASE_DIR = Path(__file__).resolve().parent.parent.parent
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BASE_DIR has no sense when client is installed in standard way (as it points somewhere to /your/path/to/python/env/lib/site-packages/)

@VukW VukW changed the title WIP: Pypi package package installation in non-editable mode: pip install ./cli Feb 29, 2024
@VukW VukW had a problem deploying to testing-external-code February 29, 2024 15:54 — with GitHub Actions Failure
Copy link
Contributor

@aristizabal95 aristizabal95 left a comment

Choose a reason for hiding this comment

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

Looks good! Although I think some discussion has to be made regarding the local server set up. Local servers are mainly for development/testing purposes, and having it packaged with the medperf client doesn't feel right.

At this point there's no easy way to set up a local server without having to go to the package folder and running the provided sh scripts. This doesn't feel like something every user needs to do and so it probably shouldn't get packaged with our installation.

This does require a lot of thinking and modifications to our resources, so it's no small decision to make. Let's have a chat soon about this!

@VukW
Copy link
Contributor Author

VukW commented Mar 18, 2024

Folks, I'm thinking. Do we need to pack local server to the pip package at all? I mean, it is used only for development purposes. What do you think if we leave it in a repo? so every developer who needs local server would have to check out repo.

VukW added 2 commits March 19, 2024 17:40
# Conflicts:
#	cli/medperf/utils.py
#	cli/requirements.txt
#	server/setup-dev-server.sh
@VukW VukW temporarily deployed to testing-external-code March 19, 2024 14:48 — with GitHub Actions Inactive
@VukW
Copy link
Contributor Author

VukW commented Mar 25, 2024

TODO:

  • return git update check
  • fetch only current branch instead of all of them?

# Conflicts:
#	cli/medperf/cli.py
#	cli/medperf/utils.py
#	cli/requirements.txt
@VukW VukW temporarily deployed to testing-external-code April 1, 2024 13:49 — with GitHub Actions Inactive
@VukW VukW temporarily deployed to testing-external-code April 1, 2024 13:52 — with GitHub Actions Inactive
@VukW VukW closed this Apr 10, 2024
@VukW VukW deleted the pypi-package branch April 10, 2024 12:08
@github-actions github-actions bot locked and limited conversation to collaborators Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants