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

Fix failure tracker #742

Merged
merged 2 commits into from Nov 3, 2017
Merged

Fix failure tracker #742

merged 2 commits into from Nov 3, 2017

Conversation

@jdm
Copy link
Member

jdm commented Oct 25, 2017

This change is Reviewable

@jdm
Copy link
Member Author

jdm commented Oct 25, 2017

This fixes a long-time issue with the intermittent-tracker where we never actually could update the installed package on the server. This also makes the new intermittent failure tracker server start up successfully; I hadn't tried it in python3 before.

@jdm
Copy link
Member Author

jdm commented Oct 25, 2017

@highfive highfive assigned metajack and unassigned aneeshusa Oct 25, 2017
Copy link
Member

aneeshusa left a comment

Seems reasonable, mostly a few questions.

@@ -9,13 +9,16 @@ intermittent-failure-tracker:
- venv_bin: virtualenv-3.5
- python: python3
- system_site_packages: False
- pip_pkgs:
- setuptools-git

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Oct 26, 2017

Member

Can you add a comment about why this is needed?

@@ -1,5 +1,5 @@
{%
set tracker = {
'rev': '37ef73bb0059e0555a903e758dae8006c46a9916'
'rev': 'f2f3825b6b026b54769a995ae8a526b2d81ee5fd'

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Oct 26, 2017

Member

It would be nice to publish all these packages to PyPI as well, as our Salt version does better with that than git installs.

This comment has been minimized.

Copy link
@jdm

jdm Oct 26, 2017

Author Member

Better in what sense?

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Oct 26, 2017

Member

#593 is what I'm aware of; this may have been fixed or improved since then but at least in our old Salt the git installs seem less well supported.

This comment has been minimized.

Copy link
@jdm

jdm Oct 26, 2017

Author Member

Per my latest comment in that issue, a2934d0 is all that is necessary. I would rather not go through the effort of publishing these packages.

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Oct 26, 2017

Member

Thanks for updating that issue, since it seems to be working then git dependencies are fine by me.

@@ -37,6 +40,12 @@ intermittent-failure-tracker:
- group: servo
- mode: 644
/home/servo/intermittent-failure-tracker/intermittent_errors.json:

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Oct 26, 2017

Member

Why do we need to create this file ourselves? Seems like something the package should create on-demand.

This comment has been minimized.

Copy link
@jdm

jdm Oct 26, 2017

Author Member

The current working directory is not owned by the user that is running the server, so it can't.

@jdm jdm force-pushed the jdm:fix-failure-tracker branch from 162a7b4 to 8926beb Oct 30, 2017
@jdm
Copy link
Member Author

jdm commented Oct 30, 2017

@metajack
Copy link
Contributor

metajack commented Oct 30, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2017

📌 Commit 8926beb has been approved by metajack

bors-servo added a commit that referenced this pull request Oct 30, 2017
Fix failure tracker

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/742)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2017

Testing commit 8926beb with merge 93a7cc7...

@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2017

💔 Test failed - status-travis

@jdm
Copy link
Member Author

jdm commented Nov 3, 2017

@bors-servo: retry

@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2017

Testing commit 8926beb with merge fd173da...

bors-servo added a commit that referenced this pull request Nov 3, 2017
Fix failure tracker

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/742)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2017

☀️ Test successful - status-travis
Approved by: metajack
Pushing fd173da to master...

@bors-servo bors-servo merged commit 8926beb into servo:master Nov 3, 2017
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
homu Test successful
Details
@jdm jdm removed the S-needs-deploy label Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.