Convert to temple #50

multigl · 2018-08-20T17:29:00Z

Temple
Circle CI 2.0

bijanvakili · 2018-08-20T17:30:11Z

temple.yaml

@@ -0,0 +1,9 @@
+_extensions: [jinja2_time.TimeExtension]
+_template: git@github.com:CloverHealth/temple-python-public.git


multigl · 2018-08-21T14:30:19Z

resolves #5 #7 #16

jiffyclub · 2018-08-21T20:22:31Z

You'll need to add installation of Postgres to the test setup (it's not part of the template), you can see an example here: https://github.com/CloverHealth/pgmock/blob/5c0d2be637c23729a306c0346b9ec1340450a54b/.circleci/config.yml#L17-L22.

6C1 · 2018-08-22T16:47:13Z

.circleci/config.yml

+  checks_and_deploy:
+    jobs:
+      - temple_check
+      - lint


- test_py34 is missing here, which causes deploy's requirements to fail below.

Or if this isn't Python 3.4 compatible, delete that job.

that seems to have done it

6C1 · 2018-08-22T19:07:15Z

temporal_sqlalchemy/tests/test_temporal_models.py

@@ -316,17 +317,16 @@ def test_disallow_flushes_within_clock_ticks_when_strict(self, session, session_
            t.prop_a = 2

            with pytest.raises(AssertionError) as excinfo:
-                eval('session.{func_name}()'.format(func_name=session_func_name))
+                eval('session.{func_name}()'.format(func_name=session_func_name))  # pylint: disable=eval-used


Instead of using eval and string interpolation, could we use getattr(session, session_fun_name)()? Here and throughout.

bijanvakili · 2018-08-22T22:09:58Z

ChangeLog

@@ -0,0 +1,2 @@
+CHANGES
+=======


Are you going to add content here from the past 10 releases?

bijanvakili · 2018-08-22T22:11:46Z

Makefile

+	pyenv install -s 3.4.4
+
+	# Set up the dev env and the environments for Tox
+	pyenv local ${MODULE_NAME} 3.6.2 3.5.3 3.4.4


Isn't pyenv local restricted to a single local version?

good question, this makefile comes from a temple public project, I think it's necessary for tox

Oh hmm I missed this in my update to the template, testing on CircleCI doesn't use tox anymore (and nor would it locally unless we set that up again).

I don't know if I'd put that effort in, tox was really challenging to get set up.

Cool, probably worth updating this in the template to only install a recent Python version.

bijanvakili · 2018-08-22T22:24:18Z

temporal_sqlalchemy/bases.py

@@ -63,13 +66,15 @@ def __init__(

    @property
    def clock_table(self):
+        """ DEPRECATED: use .clock_model instead -- Clock Model for this Model"""


Suggestion (for later if necessary): Consider using decorators or direct warnings for deprecated functionality:
https://docs.python.org/3/library/warnings.html#warnings.warn
https://stackoverflow.com/questions/2536307/decorators-in-the-python-standard-lib-deprecated-specifically

I prefer these because pylint picks them up and shows me them immediately in my IDE so I know to avoid using them.

pycharm picks up the warnings.warn on the next line about deprecation

Woops... I missed that line. Thanks!

bijanvakili · 2018-08-22T22:42:30Z

temporal_sqlalchemy/tests/test_temporal_models.py


            assert re.match(
                r'.*flush\(\) has triggered for a changed temporalized property outside of a clock tick.*',
-                str(excinfo)
+                str(excinfo),


Nitpick: I usually use str(excinfo.value) to get only the exception class rather than the pytest ExceptionInfo wrapper which sometimes doesn't include the specific message.

bijanvakili · 2018-08-22T22:43:42Z

test_requirements.txt

+pytest-mock==1.10.0
+pytest==3.7.2
+pytest-randomly>=1.0,<2
+testing.postgresql>=1.3.0


YAAAAAS... so much prefer the disposable database approach of testing.postgres 👍

bijanvakili

LGTM

…emporal-sqlalchemy into convert-to-temple

Joey Leingang added 2 commits August 20, 2018 11:26

temple scaffolding

8609290

starting flak8 and pylint fixes

16e8ef2

multigl added enhancement help wanted wip labels Aug 20, 2018

multigl requested review from gwax, bijanvakili and dave-clover August 20, 2018 17:29

bijanvakili reviewed Aug 20, 2018

View reviewed changes

multigl and others added 4 commits August 20, 2018 23:04

make validate passes

d574580

tests pass now locally

28898d3

gitignore from temple

7fd8b53

woops accidentally added a bunch of doc builds

9db53a3

lol nice typo joey

5db92b7

multigl closed this Aug 21, 2018

multigl deleted the convert-to-temple branch August 21, 2018 21:17

multigl restored the convert-to-temple branch August 21, 2018 21:17

multigl deleted the convert-to-temple branch August 21, 2018 21:17

multigl restored the convert-to-temple branch August 21, 2018 21:19

multigl reopened this Aug 21, 2018

empty commit to trigger rebuild

82d6cdb

6C1 suggested changes Aug 22, 2018

View reviewed changes

Joey Leingang added 3 commits August 22, 2018 12:51

add test_py34 to jobs workflow

43b868b

add postgres and older flake8 requirement

4c9a3e1

remove bugbear from 3.4 build

82af3f5

multigl requested review from bijanvakili and removed request for gwax and dave-clover August 22, 2018 18:21

multigl requested a review from 6C1 August 22, 2018 18:21

drop python 3.4 support

5e55e49

6C1 reviewed Aug 22, 2018

View reviewed changes

remove eval calls

a00b82a

bijanvakili reviewed Aug 22, 2018

View reviewed changes

bijanvakili approved these changes Aug 22, 2018

View reviewed changes

Joey Leingang added 2 commits August 23, 2018 13:58

back fill ye ol changelog

e3d1f1a

Merge branch 'convert-to-temple' of https://github.com/CloverHealth/t…

7b09980

…emporal-sqlalchemy into convert-to-temple

multigl merged commit d295d22 into master Aug 23, 2018

multigl deleted the convert-to-temple branch August 23, 2018 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert to temple #50

Convert to temple #50

multigl commented Aug 20, 2018 •

edited

Loading

bijanvakili Aug 20, 2018

multigl commented Aug 21, 2018

jiffyclub commented Aug 21, 2018

6C1 Aug 22, 2018

jiffyclub Aug 22, 2018

multigl Aug 22, 2018

6C1 Aug 22, 2018

bijanvakili Aug 22, 2018

multigl Aug 22, 2018

bijanvakili Aug 22, 2018

multigl Aug 22, 2018

jiffyclub Aug 22, 2018

multigl Aug 23, 2018

jiffyclub Aug 23, 2018

bijanvakili Aug 22, 2018 •

edited

Loading

multigl Aug 22, 2018

bijanvakili Aug 22, 2018

bijanvakili Aug 22, 2018

bijanvakili Aug 22, 2018

multigl Aug 22, 2018

bijanvakili left a comment

		@@ -0,0 +1,9 @@
		_extensions: [jinja2_time.TimeExtension]
		_template: git@github.com:CloverHealth/temple-python-public.git

		@@ -0,0 +1,2 @@
		CHANGES
		=======

Convert to temple #50

Convert to temple #50

Conversation

multigl commented Aug 20, 2018 • edited Loading

Choose a reason for hiding this comment

multigl commented Aug 21, 2018

jiffyclub commented Aug 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bijanvakili Aug 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bijanvakili left a comment

Choose a reason for hiding this comment

multigl commented Aug 20, 2018 •

edited

Loading

bijanvakili Aug 22, 2018 •

edited

Loading