Paths

Table of Contentst

Differential D3270

Introduce some scaffolding for an attrs-based BaseSchedulerModel
ClosedPublic
Actions

Authored by olasd on Jun 12 2020, 11:20 AM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDSCHc509a12df5f1: Introduce some scaffolding for an attrs-based BaseSchedulerModel

Summary

Alongside swh.model.model, this allows us to define data models for the objects
the scheduler is working with, and to serialize/deserialize these objects
transparently at the RPC layer.

Test Plan

no specific tox tests added; these will come when actual models will
happen in further commits

Diff Detail

Repository

rDSCH Scheduling utilities

Branch

feature/scheduler-model

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 12806
Build 19488: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 19487: arc lint + arc unit

Event Timeline

olasd created this revision.Jun 12 2020, 11:20 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 12 2020, 11:20 AM

olasd added a child revision: D3271: Implement basic storage of lister information.Jun 12 2020, 11:22 AM

Build is green

Patch application report for D3270 (id=11594)

Rebasing onto 4c0c37bab0...

Current branch diff-target is up to date.

Changes applied before test

commit 2eb7a2e07197faa5423b47d7f08111e553db0c71
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Jun 12 11:03:26 2020 +0200

    Introduce some scaffolding for an attrs-based BaseSchedulerModel
    
    Alongside swh.model.model, this allows us to define data models for the objects
    the scheduler is working with, and to serialize/deserialize these objects
    transparently at the RPC layer.

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/18/ for more details.

Harbormaster completed remote builds in B12799: Diff 11594.Jun 12 2020, 11:23 AM

This looks fine to me.

One question above.

swh/scheduler/model.py
26	I gather that's a generic definition declaration for the future scheduler model classes... Oh is that to ensure the order of the columns is always the same?

This revision is now accepted and ready to land.Jun 12 2020, 11:54 AM

Improve the ORM-like logic and add tests to it

Build is green

Patch application report for D3270 (id=11601)

Rebasing onto 4c0c37bab0...

Current branch diff-target is up to date.

Changes applied before test

commit c6054ae5f029f1c5c73120bb7252e7968f81b9fc
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Jun 12 11:03:26 2020 +0200

    Introduce some scaffolding for an attrs-based BaseSchedulerModel
    
    Alongside swh.model.model, this allows us to define data models for the objects
    the scheduler is working with, and to serialize/deserialize these objects
    transparently at the RPC layer.
    
    This also introduces some mild ORM-like logic so we can keep the actual SQL a
    little DRYer.

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/20/ for more details.

Harbormaster completed remote builds in B12806: Diff 11601.Jun 12 2020, 12:12 PM

olasd requested review of this revision.Jun 12 2020, 1:12 PM

olasd marked an inline comment as done.

olasd added inline comments.

swh/scheduler/model.py
26	Yeah. I've added a docstring and some tests to clarify the intent.

douardda added a subscriber: douardda.Jun 12 2020, 2:07 PM

douardda added inline comments.

swh/scheduler/api/serializers.py
17	don't we want to move towards an `object_type` attribute here (same as D3152)?
swh/scheduler/model.py
34	argh, one more metadata field! :-)
swh/scheduler/tests/test_model.py
23	What's the reason for this property (in the context of the test I mean)?

olasd marked an inline comment as done.Jun 12 2020, 2:28 PM

olasd added inline comments.

swh/scheduler/api/serializers.py
17	Possibly. But that won't help for the `DECODERS` entry
swh/scheduler/model.py
34	This one comes from `attrs`, I didn't do it!
swh/scheduler/tests/test_model.py
23	Making sure random properties don't show up in the SQL, only actual fields. I'll add a docstring.

douardda added inline comments.Jun 12 2020, 2:31 PM

swh/scheduler/tests/test_model.py
23	isn't this a "poor man's test" for `attr.fields()`?

douardda added inline comments.Jun 12 2020, 2:56 PM

swh/scheduler/api/serializers.py
17	ok, since this is for now completely circumscribed(?) in the scheduler, we can postpone this discussion for a later time if necessary, I guess.

Naming is hard, i proposed something that might please you (or not :)

swh/scheduler/model.py
35	db_now_at_insert (shorter db_insert_now but less clear) as a proposed replacement for auto_now_add? I found auto_now and auto_now_add distinction hard to keep in mind
37	client_now_at_upsert?

Oh and i'm fine with this otherwise, thanks.

The tests helped me a bit :)

This revision is now accepted and ready to land.Jun 12 2020, 5:50 PM

olasd added inline comments.Jun 15 2020, 12:48 PM

swh/scheduler/model.py
35	I'd much much rather re-use the exact django field terminology than invent something else, because that would be confusing too.
swh/scheduler/tests/test_model.py
23	Maybe, yes.

Apply some comments

Build is green

Patch application report for D3270 (id=11631)

Rebasing onto 4c0c37bab0...

Current branch diff-target is up to date.

Changes applied before test

commit c509a12df5f1c25be3e492c693be11eae382862a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Jun 12 11:03:26 2020 +0200

    Introduce some scaffolding for an attrs-based BaseSchedulerModel
    
    Alongside swh.model.model, this allows us to define data models for the objects
    the scheduler is working with, and to serialize/deserialize these objects
    transparently at the RPC layer.
    
    This also introduces some mild ORM-like logic so we can keep the actual SQL a
    little DRYer.

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/24/ for more details.

Harbormaster completed remote builds in B12838: Diff 11631.Jun 15 2020, 12:59 PM

olasd added a child revision: D3283: Introduce a SchedulerException base class.Jun 15 2020, 3:00 PM

olasd removed a child revision: D3271: Implement basic storage of lister information.Jun 15 2020, 3:01 PM

Closed by commit rDSCHc509a12df5f1: Introduce some scaffolding for an attrs-based BaseSchedulerModel (authored by olasd). · Explain WhyJun 15 2020, 4:02 PM

This revision was automatically updated to reflect the committed changes.

olasd added a commit: rDSCHc509a12df5f1: Introduce some scaffolding for an attrs-based BaseSchedulerModel.