Made UTC timezone explicit in common/init.sls. #439

ghost · 2016-07-15T17:49:17Z

Relevant to #303.

This change is


        Made UTC timezone explicit in common/init.sls.

aneeshusa · 2016-07-15T17:55:58Z

common/init.sls

@@ -40,6 +40,9 @@ servo:
    - shell: /bin/bash
    - home: {{ common.servo_home }}

+Etc/UTC:


This can just be UTC.

aneeshusa · 2016-07-15T17:59:41Z

Please also add a test for this. I think running the date command and ensuring UTC is in the output will work.

Add a new directory tests/sls/common and put the test in a new file tests/sls/common/timezone.py. Also, please make an empty tests/sls/common/__init__.py file, and update the .travis/dispatch.sh code to run the test on all nodes.


        fixup! Made UTC timezone explicit in common/init.sls.


        Added test for UTC timezone.

ghost · 2016-07-15T18:34:01Z

I've added the test and simplified to just UTC.

aneeshusa · 2016-07-15T18:36:58Z

.travis/dispatch.sh

@@ -53,6 +53,6 @@ else
    # Only run tests against the new configuration
    # TODO: don't hard-code this
    if [[ "${SALT_NODE_ID}" == "servo-master1" ]]; then
-        ./test.py sls.buildbot.master sls.homu sls.nginx
+        ./test.py sls.buildbot.master sls.common.timezone sls.homu sls.nginx


Two things:

this can be just sls.common, all of the test files in that dir will get run automatically.

we want to run the common tests on all nodes, not just servo-master1, so that should happen outside the conditional.

aneeshusa · 2016-07-15T18:37:33Z

tests/sls/common/timezone.py

+
+
+def run():
+    command = "date | grep -v UTC"


Instead of using pipes and an extra grep process, let's just look for UTC in the output of the date command inside Python.

aneeshusa · 2016-07-15T18:38:06Z

tests/sls/common/timezone.py

+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE,
+                         universal_newlines=True,
+                         shell=True)


Please remove shell=True, it can cause security problems (and also spawns an extra process).

KiChjang · 2016-08-01T22:55:16Z

@RowanEB Are you still planning on finishing this up?

charlesvdv · 2016-12-29T14:37:08Z

@aneeshusa @KiChjang I'm interested in continuing the work of @RowanEB if that's ok for you ? (as he doesn't respond in a long time)

aneeshusa · 2016-12-29T22:32:55Z

@charlesvdv please go ahead!


        Auto merge of #569 - charlesvdv:utc, r=aneeshusa

Use UTC timezone Fix the issue #303 and follow up of #439 PR. I'm not sure about the spot to put the ```sls.common``` tests.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/569)

Made UTC timezone explicit in common/init.sls.

Loading status checks…

80b816a

aneeshusa reviewed Jul 15, 2016
View changes

common/init.sls Outdated

@@ -40,6 +40,9 @@ servo:

- shell: /bin/bash

- home: {{ common.servo_home }}

Etc/UTC:

This comment has been minimized.

Sign in to view

aneeshusa Jul 15, 2016
Member

This can just be UTC.

Rowan Bulkow added 2 commits Jul 15, 2016

fixup! Made UTC timezone explicit in common/init.sls.

c4dc7ab

Added test for UTC timezone.

Loading status checks…

8e4014e

aneeshusa reviewed Jul 15, 2016
View changes

tests/sls/common/timezone.py

def run():

command = "date | grep -v UTC"

This comment has been minimized.

Sign in to view

aneeshusa Jul 15, 2016
Member

Instead of using pipes and an extra grep process, let's just look for UTC in the output of the date command inside Python.

aneeshusa reviewed Jul 15, 2016
View changes

tests/sls/common/timezone.py

stdout=subprocess.PIPE,

stderr=subprocess.PIPE,

universal_newlines=True,

shell=True)

This comment has been minimized.

Sign in to view

aneeshusa Jul 15, 2016
Member

Please remove shell=True, it can cause security problems (and also spawns an extra process).

aneeshusa self-assigned this Jul 18, 2016

aneeshusa added the S-needs-code-changes label Jul 18, 2016

aneeshusa mentioned this pull request Dec 29, 2016

Use UTC Timezone #303

Closed

KiChjang closed this Dec 30, 2016

charlesvdv mentioned this pull request Dec 30, 2016

Use UTC timezone #569

Merged

servo / saltfs

Made UTC timezone explicit in common/init.sls. #439

Made UTC timezone explicit in common/init.sls. #439

ghost commented Jul 15, 2016 •

edited by larsbergstrom

This comment has been minimized.

aneeshusa commented Jul 15, 2016

ghost commented Jul 15, 2016

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

KiChjang commented Aug 1, 2016

charlesvdv commented Dec 29, 2016

aneeshusa commented Dec 29, 2016

servo / saltfs

Join GitHub today

Made UTC timezone explicit in common/init.sls. #439

Made UTC timezone explicit in common/init.sls. #439

Conversation

ghost commented Jul 15, 2016 • edited by larsbergstrom

This comment has been minimized.

aneeshusa Jul 15, 2016

aneeshusa commented Jul 15, 2016

ghost commented Jul 15, 2016

This comment has been minimized.

aneeshusa Jul 15, 2016

This comment has been minimized.

aneeshusa Jul 15, 2016

This comment has been minimized.

aneeshusa Jul 15, 2016

KiChjang commented Aug 1, 2016

charlesvdv commented Dec 29, 2016

aneeshusa commented Dec 29, 2016

ghost commented Jul 15, 2016 •

edited by larsbergstrom