-
Notifications
You must be signed in to change notification settings - Fork 1
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
optimake serve
#58
optimake serve
#58
Conversation
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.
Like the approach so far @eimrek! Env vars is probably the way to go for server config for now, but we could definitely make that easier on the o-p-t side (e.g., add passable config objects everywhere that default to the global one).
Am I okay to merge Materials-Consortia/optimade-python-tools#2094 now then? Can do a release too so its easier for you to test out.
I just merged and released Materials-Consortia/optimade-python-tools#2094 if you want to try it out @eimrek! Added a few tests and made it slightly more ergonomic (allow absolute and relative paths etc), but it should allow for the same invocation as before. |
thanks @ml-evs, will have a test run soon. But just to double-check, would it make sense to add something like optimade-maker/src/optimake/serve.py Line 46 in 556fecc
to optimade python tools instead? Basically, just so if you pass in a jsonl file, it also takes the provider fields from there. And possibly ignores the provider fields from the config files. Happy to move this to o-p-t, if there is a clear way to do it. |
Yeah I think we can consider that now, just didn't want to hold you up on this PR. It would be doable, but I think essentially the JSONL loading process will have to update the global CONFIG object for this to work smoothly for now. I can have a play around with it too. |
resolves #45
This PR adds the
optimake serve
command thatoptimade.jsonl
;optimade.jsonl
.Uses the
insert_from_jsonl
feature from the PR Materials-Consortia/optimade-python-tools#2094Couple of comments:
optimade-python-tools
server. I ended up just passing the config through environment variables.optimade-python-tools
instead, considering we're passing in the jsonl anyways.Note: I also changed the default
optimake
command tooptimake convert
.@ml-evs does this approach make sense? if you agree, i'll try to think how to add tests for this as well.