#3067 Depend on zmq and not all of zarkov

v1.0.0
closed
Rick Copeland
cleanup (28) sf-2 (994)
General
Cory Johns
2015-08-20
2011-10-21
Dave Brondsema
No

Zarkov pulls in a lot of deps but Allura only needs the zarkov client which is a very thin wrapper around zmq. We should just use zmq directly. We can also remove the ImportError handling and always require zmq. The ini config option can stay.

Discussion

  • Dave Brondsema

    Dave Brondsema - 2012-01-06
    • size: --> 2
     

  • Rick Copeland - 2012-03-13
    • status: open --> code-review
    • assigned_to: Rick Copeland ☕
    • qa: Dave Brondsema
     

  • Rick Copeland - 2012-03-13

    Code is checked in that removes gevent, zarkov, etc. from requirements.txt and refactors import checking to look for zmq rather than zarkov. on rc/3067

     

  • Dave Brondsema

    Dave Brondsema - 2012-03-14
    • qa: Dave Brondsema --> Cory Johns
     

  • Dave Brondsema

    Dave Brondsema - 2012-03-14

    We need to do additional testing on sandboxes, since Rick doesn't yet have access to do that, and only tested locally.

     

  • Cory Johns - 2012-03-14

    He just copied the ZarkovClient into Allura, so I'm confident that, if the tests pass, everything will be fine.

     

  • Cory Johns - 2012-03-14
    • status: code-review --> open
     

  • Cory Johns - 2012-03-14

    Per Dave, the test suite doesn't enable zarkov events and there are no explicit tests of the zarkov-enabled code path. Please add a test for this code path.

     

  • Dave Brondsema

    Dave Brondsema - 2012-03-14

    I'm not 100% that we don't have a test covering that path. But I don't remember one.

     

  • Rick Copeland - 2012-03-15
    • status: open --> code-review
     

  • Rick Copeland - 2012-03-15

    Tests for all zarkov_helpers have now been added in rc/3067

     

  • Cory Johns - 2012-03-19

    Looks good, and merged to dev, but I have a couple of notes:

    -        start_date (datetime or str): Start of the date range. If a str is
-            passed, it must be in %Y-%m-%d format.
-        end_date (datetime or str): End of the date range. If a str is
-            passed, it must be in %Y-%m-%d format.
+        start_date (datetime): Start of the date range.
+        end_date (datetime or str): End of the date range.

    I'm guessing that last (datetime or str) should be (datetime).

    Also, the TestZeroFill tests, while very good, don't give much info on debugging them should they start failing. I think an ideal test would include comments or such to explain where the magic numbers in the assertEquals come from and why those values are expected. But it's obviously not critical, and if the tests break, hopefully the person who broke them will have at least a fair idea of why their change is impacting those tests.

    • status: code-review --> closed
     

Log in to post a comment.