Skip to content

Conversation

@stepanstipl
Copy link
Contributor

@stepanstipl stepanstipl commented Oct 19, 2018

This PR moves dataloader from original gpii-ops/gpii-dataloader repo into universal.

Main reasons behind this:

  • Get rid of the duplication of Dockerfile and necessity to maintain extra image and pipeline
  • Get rid of cloning the repo and installing npm modules when the container is executed (this would be slow, is prone to errors and would raise question of versioning)
  • Data loader will always have the new universal code (as it will be running in the universal image)
  • Data loader will get triggered every time there's a change to universal (as the universal image will get updated)

Most of the code was in universal already, and gpii-ops/gpii-dataloader contained basically only shell wrapper and the docker image. Work on new dataloader (GPII-3138 and #626) requires changes to the existing dataloader wrapper and therefore presented a good opportunity for the move.

This is a continuation of gpii-ops/gpii-dataloader#6 PR.

Main changes:

  • Import deleteAndLoadSnapsets.sh and DataLoader.md
  • Add dataLoader dependencies to docker image (jq and curl)
  • Update data loader script to reflect new location in universal
  • Minor formatting changes
  • Add DB connectivity check to dataloader
  • Update vagrantCloudBasedContainers.sh to work with new dl location
  • Update DataLoader docs to reflect current state

I have tested this locally and plan to create corresponding PR and test for gpii-infra changes required.

@gpii-bot
Copy link

Could one of the admins verify that these changes are reasonable to test? If so, please reply with "ok to test".

@cindyli
Copy link
Contributor

cindyli commented Oct 19, 2018

ok to test

@gpii-bot
Copy link

CI job failed: https://ci.gpii.net/job/universal-tests/1212/

@gpii-bot
Copy link

CI job passed: https://ci.gpii.net/job/universal-tests/1214/

Copy link
Contributor

@mrtyler mrtyler left a comment

Choose a reason for hiding this comment

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

LGTM

# components: the Preferences Server and the Flow Manager.
#
# It also starts a CouchDB container and loads the CouchDB data into
# It also starts a CouchDB container and data into
Copy link
Contributor

Choose a reason for hiding this comment

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

Old text was correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this, not sure how it got in and I didn't notice, reverted back to original text.

@gpii-bot
Copy link

CI job passed: https://ci.gpii.net/job/universal-tests/1226/

@gpii-bot
Copy link

CI job failed: https://ci.gpii.net/job/universal-tests/1229/

@stepanstipl
Copy link
Contributor Author

I'm closing this in favor of #692 (via klown#3) as we agreed with @klown

@gpii-bot
Copy link

CI job passed: https://ci.gpii.net/job/universal-tests/1231/

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.

4 participants