Skip to content

Conversation

rentzso
Copy link
Contributor

@rentzso rentzso commented Mar 11, 2016

closes #82

to bootstrap data I needed to define the env variable before bootstrap: ce1ba92

I am not sure if that is the right way. @ryansanford @gsfr any suggestions?

@kofalt
Copy link
Contributor

kofalt commented Mar 11, 2016

The diff makes sense so far... but I want to make sure I understand correctly.

Gunnar mentioned to us earlier about how this will start to cause loopback resolution, and I see that in the code. Can you walk us through the path you travelled? I'm not entirely opposed to this landing, but when it does I'd like to understand what we can do in the future to avoid loopback calls.

@ryansanford
Copy link
Contributor

@rentzso
manually adding ENV vars should not be required when going through run-dev.sh.

Change to docker/uwsgi-entrypoint.sh should be backed out. See https://github.com/scitran/core/blob/master/docker/README.md for example on how to bootstrap in a raw scitran/core env and not the flywheel dev environment. If there's a new ENV var that is required, those instructions should be updated, and the ENV var should be added to sample.config

@rentzso
Copy link
Contributor Author

rentzso commented Mar 11, 2016

@ryansanford
Thanks for the info.
What changes are needed in fly/fly after adding the required ENV in the scitran/core?

@kofalt
Something that I have considered, but excluded quickly is to continue to load the schemas from the filesystem while serving them to external clients from the API.
This is problematic as it may create inconsistencies between two schemas. Also I have noticed some weirdness in json schemas resolution from the filesystem.

A possible alternative to avoid the loopback would be to statically serve schemas, maybe on a different endpoint. The downside of this is that we will have always to hardcode all the schemas. Serving them in the API would allow us in the future to create/modify them on the fly. Not sure if this will be needed but it is nice to have.

@ryansanford
Copy link
Contributor

@rentzso
In fly/fly, add it to docker/dev.env. All vars there get consumed by scitran/core containers.

@rentzso
Copy link
Contributor Author

rentzso commented Mar 11, 2016

@ryansanford I have added the env variable in fly/fly and update the docs/sample.config
@kofalt Is it ok for you to merge or do you need more time to review the code?

@rentzso
Copy link
Contributor Author

rentzso commented Mar 11, 2016

@ryansanford I have tried to build the docker containers as in https://github.com/scitran/core/blob/master/docker/README.md
everything was fine except bootstrap-data.sh

I have got this error:

Collecting git+https://github.com/scitran/reaper.git@30215c66a33b18685e1608dbe952e78c370d8765
  Cloning https://github.com/scitran/reaper.git (to 30215c66a33b18685e1608dbe952e78c370d8765) to /tmp/pip-xfTF7O-build
fatal: unable to look up current user in the passwd file: no such user

It seems not possible to clone the reaper repo. Can you check it? Thanks!

@ryansanford
Copy link
Contributor

@rentzso
I just merged a correction to the bootstrap example in docker/README.md, which should resolve your error. Please try again using this updated command.

# Bootstrap Data Example:
   docker run \
     -e "SCITRAN_RUNTIME_HOST=scitran-core" \
     -e "SCITRAN_RUNTIME_PORT=8080" \
     -e "SCITRAN_RUNTIME_PROTOCOL=http" \
     -e "SCITRAN_CORE_DRONE_SECRET=change-me" \
     -e "PRE_RUNAS_CMD=/var/scitran/code/api/docker/bootstrap-data.sh" \
     --link scitran-core \
     --volumes-from scitran-core \
     --rm \
     scitran-core \
       echo "Data bootstrap complete."

@kofalt
Copy link
Contributor

kofalt commented Mar 14, 2016

@rentzso Are the only differences you've observed regarding $ref?

Can you contrast for me a code snippet that loads a JSON schema from a URL, versus a snippet that loads from a file? I'd like to play with this to see if I can make headway.

@rentzso
Copy link
Contributor Author

rentzso commented Mar 14, 2016

You can find one of the "weirdness" that I was forced to use for $ref in input/download.json:
you will see two references to subschemas defined in the same document, the only way to make them work was for the ones at line 51-52 to reference the same download.json schema and at line 8-9 NOT to reference it.
After serving the schemas this should become more coherent.

@rentzso
Copy link
Contributor Author

rentzso commented Mar 14, 2016

@ryansanford I have tested with the new scripts and also data creation works.
@kofalt let me know if you need to give more thought about the issue or if I can merge.
Thanks!

@kofalt
Copy link
Contributor

kofalt commented Mar 14, 2016

@rentzso: Fine to merge on my end pending others. We can fiddle with it later.

rentzso added a commit that referenced this pull request Mar 14, 2016
@rentzso rentzso merged commit a2e402c into master Mar 14, 2016
@rentzso rentzso deleted the serve-schemas branch March 14, 2016 19:45
ryansanford added a commit that referenced this pull request May 18, 2016
This variable was a temporary measure from #190 and never fully removed until now.
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.

create schema subroutes

3 participants