Skip to content

Conversation

@gregjan
Copy link
Member

@gregjan gregjan commented Sep 24, 2018

Includes a command.sh script that includes a migratedb and then server startup. There might be a better way to arrange that. For instance a docker-compose.yml file might specify a command that only starts the server.
I've added the commit ID as an image label. I'm mostly doing this as a way of tracking specific code with respect to performance test metrics. Should allow deeper investigation of outcomes through diffs.

Copy link
Member

@acoburn acoburn left a comment

Choose a reason for hiding this comment

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

I don't know a lot about Docker images, but other than some very minor changes, this all looks good to me.

@acoburn
Copy link
Member

acoburn commented Sep 24, 2018

Strangely, when I use this branch locally on JDK11, the grgit plugin seems to trigger a failure:

No .git directory found!

That's when running gradle clean (or any gradle command). It works fine on JDK8, even though the ./git directory is present in both environments. It also seemed to work fine on Travis-CI under JDK 11. Any ideas?

This also makes me think that a direct download (rather than clone) of the git repo wouldn't have the ./git directory, so maybe depending on the presence of a .git directory might cause problems later? Or maybe wrap the call in some sort of try ... catch block?

@gregjan
Copy link
Member Author

gregjan commented Sep 24, 2018 via email

@gregjan
Copy link
Member Author

gregjan commented Sep 24, 2018 via email

@acoburn
Copy link
Member

acoburn commented Sep 24, 2018

Yes, the rest of the code works fine on JDK11

@acoburn
Copy link
Member

acoburn commented Sep 24, 2018

@gregjan I got this to work on my local JDK 11 environment:

docker {
   ...
   def gitLabels = ['git.url':'https://github.com/trellis-ldp/trellis-ext-db']
   try {
       git = grgit.open()
       gitLabels.put('git.commit', git.head().id);
   } catch (all) {
   }
   labels(gitLabels)
   ...
}

(I moved the grgit code from the ext { ... } block to the docker { ... } block)

@acoburn acoburn merged commit 873587a into trellis-ldp:master Sep 24, 2018
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