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

Update install.pp #106

Closed
wants to merge 4 commits into from
Closed

Update install.pp #106

wants to merge 4 commits into from

Conversation

nrvale0
Copy link

@nrvale0 nrvale0 commented Dec 31, 2013

At least for me, bundle install was unreliable because of timeouts resolving / connecting to rubygems.org. The exec appears to be safe for multiple passes so I added a 'tries' attribute.

nrvale0 added 2 commits December 31, 2013 13:03
At least for me, bundle install was unreliable because of timeouts resolving / connecting to rubygems.org. The exec appears to be safe for multiple passes so I added a 'tries' attribute.
Even more retries to work around unreliable rubygems.org DNS.
@atomaka
Copy link
Collaborator

atomaka commented Dec 31, 2013

Although I haven't had this happen with puppet-gitlab specifically, I have been running into this issue over the last several months with rubygems.org. Solution is hacky but seems reasonable enough.

I'll update once I get a chance to see why tests are failing.

@atomaka
Copy link
Collaborator

atomaka commented Dec 31, 2013

@nrvale0 Please update to pass puppet-lint tests. We ignore 80 chars, variable scope, and params class inheritance warnings.

@nrvale0
Copy link
Author

nrvale0 commented Dec 31, 2013

More patches on the way. BTW, necessity of these hacks appears to be due to confluence of rubygems.org DNS reliability and Virtualbox NAT DNS caching. thx for the merge.

nrvale0 added 2 commits December 31, 2013 14:06
* massage for puppet-lint
* add some bundle options to work around failures + perhaps speed things up a little.
@igalic
Copy link
Collaborator

igalic commented Jan 2, 2014

The generally accepted solution for rubygems going down is to setup a local rubygems mirror.
Might be overkill if gitlab is your only ruby project.
But then again, you're using puppet, so it's at least two. (still might seem unreasonable ;)

cwd => "${git_home}/gitlab",
unless => 'bundle check',
timeout => 0,
tries => 10,
Copy link
Owner

Choose a reason for hiding this comment

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

You re-tries the puppet exec command 10x and bundle re-tries 5x, so in the worst case this command is executed 50x, maybe the good solution here, is to fail because we encounter a network/transversal issue no ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Failing vocally with a good error message might be a better solution than spamming bundle install and hoping we make it through the Gemfile before we pass our max tries.

Copy link
Owner

Choose a reason for hiding this comment

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

@sbadia
Copy link
Owner

sbadia commented Jan 22, 2014

@nrvale0 please, can you squash your commits in one ?

sbadia added a commit that referenced this pull request Mar 28, 2014
As said Andrew, we have a large number of users come here expecting
the application to handle a full installation of the GitLab software.
This commit manage this use-case, but with the Igor idea we can easily
disable this « feature »

Refs: #111, #109, #106, #35
@sbadia
Copy link
Owner

sbadia commented Apr 3, 2014

Hi, ping! Any update on this PR ? :-)

@nrvale0
Copy link
Author

nrvale0 commented Apr 3, 2014

Re being less aggressive on the retries: the purpose of the module is to get gitlab installed. In this case my own experience and the experience of other folks with whom I've spoken has been that rubygems.org DNS issues are not uncommon. In my mind, being less aggressive as to the number of retries doesn't move us toward the end solution of having gitlab installed and thus I lean toward leaving the ridiculous number of retries in the code.

As to squashing the commits, I'm not sure how to do that but I will investigate. If you have a doc/reference you can share that would help.

Thx!

@atomaka
Copy link
Collaborator

atomaka commented Apr 3, 2014

@nrvale0 man git-rebase for the command used to squash commits. http://git-scm.com/book/en/Git-Tools-Rewriting-History also provides an overview for rewriting history.

@nrvale0
Copy link
Author

nrvale0 commented Apr 3, 2014

@atomaka : I rebased + squashed the feature branch and did a git push w/ force to my forked repo and it looks to have squashed all of the commits. Verifying that it now shows up as squashed in the PR....

@igalic
Copy link
Collaborator

igalic commented Apr 3, 2014

@nrvale0 you'll have to git push --force to publish it in this space.

@nrvale0
Copy link
Author

nrvale0 commented Apr 5, 2014

After the pull to master from canonical repo and rebase of the feature branch I'm uncomfortable submitting this PR without additional review and testing. I'll review and perhaps resubmit later.

@nrvale0 nrvale0 closed this Apr 5, 2014
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