Page MenuHomeSoftware Heritage
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
swh-public-ci added a comment.Jun 12 2020, 11:23 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
ardumont accepted this revision.Jun 12 2020, 11:54 AM
ardumont added a subscriber: ardumont.

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
olasd updated this revision to Diff 11601.Jun 12 2020, 12:10 PM

Improve the ORM-like logic and add tests to it

swh-public-ci added a comment.Jun 12 2020, 12:12 PM

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.

ardumont added a comment.Jun 12 2020, 5:47 PM

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?

ardumont accepted this revision.Jun 12 2020, 5:50 PM

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.

olasd updated this revision to Diff 11631.Jun 15 2020, 12:56 PM

Apply some comments

swh-public-ci added a comment.Jun 15 2020, 12:59 PM

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.

Revision Contents
Changeset List

PathSize
1 line
swh/
scheduler/
api/
4 lines
28 lines
4 lines
57 lines
tests/
77 lines

Diff 11649

View Options

requirements.txt

Loading...
View Options

swh/scheduler/api/client.py

Loading...
View Options

swh/scheduler/api/serializers.py

Loading...
View Options

swh/scheduler/api/server.py

Loading...
View Options

swh/scheduler/model.py

Loading...
View Options

swh/scheduler/tests/test_model.py

Loading...
About · Developer information · Legal