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

fix: revert to setting PYTHONPATH on the composefile for dev #294

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

sergiolaverde0
Copy link
Contributor

This commit reverts the process of setting the PYTHONPATH on the dev environment to being done on the composefile instead of doing it during the build process.

This commit reverts the process of setting the PYTHONPATH on the dev
environment to being done on the composefile instead of doing it during
the build process.
@simjanos-dev
Copy link
Owner

simjanos-dev commented Jun 3, 2024

Thank you!

Can you please explain 2 things:

  • What is the difference between creating the env variable in the dockerfile and the docker compose yml file, what caused the problem?
  • If I understand it correctly, this overwrites the PYTHONPATH, which tells python where to look for packages. Does python have some default paths, that you cannot remove, and this is the reason it looks both in the models folder and in the default python install folder?

@sergiolaverde0
Copy link
Contributor Author

  • I'm assuming that the issue is trying to set the variable to some location that doesn't exist yet, so moving it to the creation of the container ensures it is correctly set to the mounted folder. Or at least that's my hypothesis, I'm not really certain.
  • By default that variable should be unset and setting it only adds extra places for Python to look for packages on top the already defined in PYTHONHOME and the library locations according to the documentation.

If this doesn't work yet I have thought of setting the variable directly inside the script itself, so let me know if any issue arises no matter how small.

@simjanos-dev
Copy link
Owner

simjanos-dev commented Jun 5, 2024

I'm assuming that the issue is trying to set the variable to some location that doesn't exist yet, so moving it to the creation of the container ensures it is correctly set to the mounted folder. Or at least that's my hypothesis, I'm not really certain.

That does make sense. But then why would deleting the storage/app/model directory fix this issue?

If this doesn't work yet I have thought of setting the variable directly inside the script itself, so let me know if any issue arises no matter how small.

I might need a few days to test this again.

@simjanos-dev simjanos-dev merged commit a7acbaa into simjanos-dev:dev Jun 6, 2024
@sergiolaverde0 sergiolaverde0 deleted the slim branch June 6, 2024 23:08
sergiolaverde0 added a commit to sergiolaverde0/LinguaCafe that referenced this pull request Jun 6, 2024
simjanos-dev added a commit that referenced this pull request Jun 7, 2024
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.

None yet

2 participants