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 EVA Configuration #323

Merged
merged 6 commits into from
Aug 13, 2022
Merged

Fix EVA Configuration #323

merged 6 commits into from
Aug 13, 2022

Conversation

LordDarkula
Copy link
Collaborator

@LordDarkula LordDarkula commented Aug 13, 2022

I was getting periodic nondeterministic test failures due to how broken the paths are on system setup.

Use pathlib library for calculating paths when starting eva_server

Building paths by appending strings is error-prone and will not work on other platforms. Python pathlib takes
care of that automatically.

Remove Python reference to python gettempdir() in bootstrap_environment

On ada-01, I do not have permission to access /tmp, so no matter what I do, the unit tests will fail periodically and need to be fixed. Since eva already creates a ~/.eva/ directory we can use /.eva/tmp as a temporary directory and clean it up in the cleanup script.

EVA_UPLOAD_DIR in dictionary.py was also unused previously which caused confusion.

@jarulraj
Copy link
Member

Important change. Is it to possible to quickly check for other hard-coded paths?

@LordDarkula
Copy link
Collaborator Author

Important change. Is it to possible to quickly check for other hard-coded paths?

There are a couple instances of hard-coded paths used in tests, but they are all used with Mocks so changing them is not really necessary. I could not find any other instances of hard-coded paths. Maybe @gaurav274 can help me on this.

@jarulraj
Copy link
Member

We should mock the local dev environment in a test case (that is why coverage is dropping a bit). Otherwise, LGTM.

@jarulraj jarulraj merged commit f9a0dab into georgia-tech-db:master Aug 13, 2022
@jarulraj jarulraj mentioned this pull request Aug 13, 2022
This pull request was closed.
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.

2 participants