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

Update readme #136

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Update readme #136

wants to merge 2 commits into from

Conversation

natlibfi-kaisa
Copy link
Contributor

Description

Updated README follows the current way the project is built.

Motivation and Context

The older version used pyenv to handle different python versions but using it had many problems. We use 3.11 so
simply creating a good old-fastioned venv with that version is enough and simplifies everything.

Some versions of dependencies to use, namely poetry and libxmlsec1 are now specified.

Docker is the recommended way to run the application so it's been put to the top.

A lot of restructuring and removing useless sections not used in E-kirjasto was done to make the file more easier to read.

How Has This Been Tested?

I've followed the steps described in the file for some time now. The reviewers should follow the steps to confirm they are also able to run the application with Docker AND run the api and core tests.

- Removes sections that are irrelevant in E-kirjasto.
- Includes changes to how the project is currently built using Docker.
- Changes related to how Python 3.11 with venv is used to run tests (or the app locally). pyenv was a headache.
Copy link
Contributor

@NatLibFi-JoonaKupe NatLibFi-JoonaKupe left a comment

Choose a reason for hiding this comment

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

I was able to have it up and running, but I did already have an install. Can't verify this with clean install.

Copy link

@natlibfi-ptalosel natlibfi-ptalosel left a comment

Choose a reason for hiding this comment

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

I tried to do the setup

  • I finished 1 Set up and run the application with Docker Compose.
  • In section 2 Python and dependencies to run tests (and the application locally) I encountered a problem: one necessary dependency (libxmlsec1) could not be installed with these README instructions.
  • I could not proceed to section 3 Testing because of the error in dependency installation

```shell
python3 --version
```sh
docker compose -f docker-compose-dev.yml up --build -d

Choose a reason for hiding this comment

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

I had to comment out data-api section in docker-compose-dev.yml, otherwise an error would appear: Error response from daemon: no matching manifest for linux/arm64/v8 in the manifest list entries: no match for platform in manifest: not found


```sh
brew install pkg-config libffi
brew install tvuotila/libxmlsec1/[email protected]

Choose a reason for hiding this comment

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

Could not be installed. Homebrew displayed this error when trying to install Error: [email protected] has been disabled because it is not supported upstream! It was disabled on 2024-10-24.

brew install pkg-config libffi
brew install libxmlsec1
brew install libjpeg
brew install [email protected]

Choose a reason for hiding this comment

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

Could not be installed via Homebrew. Instead, I installed first pipx via Homebrew, then pipx install poetry==1.8.3. I also had to add Poetry directory to PATH environment.

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

Successfully merging this pull request may close these issues.

3 participants