Paths

Table of Contentst

Differential D8058

[docs] Expand section on testing sensors
ClosedPublic
Actions

Authored by • cdecarolis on May 24 2021, 7:26 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Sep 21, 2:56 PM

	Unknown Object (File)
	Tue, Sep 13, 9:07 AM

	Unknown Object (File)
	Tue, Sep 13, 9:07 AM

	Unknown Object (File)
	Tue, Sep 13, 9:07 AM

	Unknown Object (File)
	Tue, Sep 13, 9:07 AM

	Unknown Object (File)
	Tue, Sep 13, 9:07 AM

	Unknown Object (File)
	Tue, Sep 13, 9:07 AM

	Unknown Object (File)
	Tue, Sep 13, 9:07 AM

View All 34 Files

Subscribers

None

Details

Reviewers

• sandyryza
• yuhan
• prha

Commits

R1:0a56501ba729: [docs] Expand section on testing sensors

Summary

This diff expands on the sensors testing section

Test Plan

docs build, unit

Screen Shot 2021-05-25 at 1.42.27 PM.png (495×938 px, 44 KB)

Screen Shot 2021-05-24 at 3.23.55 PM.png (649×884 px, 110 KB)

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• cdecarolis created this revision.May 24 2021, 7:26 PM

Harbormaster completed remote builds in B31076: Diff 38219.May 24 2021, 7:49 PM

• cdecarolis requested review of this revision.May 24 2021, 7:49 PM

so good! it could close https://github.com/dagster-io/dagster/issues/3845 right?

• yuhan added inline comments.May 24 2021, 9:51 PM

examples/docs_snippets/docs_snippets/concepts/partitions_schedules_sensors/sensors/sensors.py
55	seems like `instance_for_test` comes from nowhere. should we either include `import instance_for_test` inside the code snippet or switch to use another func to build an ephemeral instance?

• yuhan mentioned this in D8054: [docs] Schedule testing section, put schedule examples under test.May 24 2021, 9:54 PM

• sandyryza requested changes to this revision.May 24 2021, 9:55 PM

• sandyryza added inline comments.

docs/content/concepts/partitions-schedules-sensors/sensors.mdx
198	Given that the sensor doesn't use the instance, it seems like unnecessary boilerplate to need to define an instance here. Are we able to not require that?

This revision now requires changes to proceed.May 24 2021, 9:55 PM

• cdecarolis added inline comments.May 25 2021, 12:44 AM

examples/docs_snippets/docs_snippets/concepts/partitions_schedules_sensors/sensors/sensors.py
55	Good point. I think sensors/schedules require persistent instances, but maybe making an example that doesn't use the instance at all will let us get away with not using one.

• sandyryza added inline comments.May 25 2021, 4:37 AM

examples/docs_snippets/docs_snippets/concepts/partitions_schedules_sensors/sensors/sensors.py
56	Does this work on master? I believe that, with Phil's recent addition of sensor cursors,`get_execution_data` now returns a `SensorExecutionData`, which I don't believe is iterable.

• cdecarolis added inline comments.May 25 2021, 1:20 PM

examples/docs_snippets/docs_snippets/concepts/partitions_schedules_sensors/sensors/sensors.py
56	yes, hence the `requests.run_requests`. A better variable name would probably help here, like `execution_data.run_requests`.

Add get_execution_data to api ref, update overview to reflect this

Harbormaster completed remote builds in B31155: Diff 38321.May 25 2021, 6:16 PM

• cdecarolis edited the test plan for this revision. (Show Details)May 25 2021, 7:35 PM

• cdecarolis added a reviewer: • prha.

• prha added inline comments.May 26 2021, 3:42 PM

python_modules/dagster/dagster/core/definitions/sensor.py
284	we should add an args entry for cursor

• cdecarolis added inline comments.May 26 2021, 3:44 PM

python_modules/dagster/dagster/core/definitions/sensor.py
284	ah good catch, thanks

get_execution_data -> evaluate_tick

Harbormaster completed remote builds in B31265: Diff 38477.May 26 2021, 7:46 PM

animated_excited_guy

• sandyryza accepted this revision.May 26 2021, 8:27 PM

This revision is now accepted and ready to land.May 26 2021, 8:27 PM

Closed by commit R1:0a56501ba729: [docs] Expand section on testing sensors (authored by Chris DeCarolis <cdguitar817@gmail.com>). · Explain WhyMay 27 2021, 2:22 PM

This revision was automatically updated to reflect the committed changes.

Chris DeCarolis <cdguitar817@gmail.com> added a commit: R1:0a56501ba729: [docs] Expand section on testing sensors.

Revision Contents
Changeset List

Path

Size

docs/

content/

api/

searchindex.json

concepts/

partitions-schedules-sensors/

18 lines

sphinx/

sections/

api/

apidocs/

schedules-sensors.rst

2 lines

examples/

docs_snippets/

docs_snippets/

concepts/

partitions_schedules_sensors/

sensors/

19 lines

docs_snippets_tests/

concepts_tests/

partitions_schedules_sensors_tests/

test_sensors.py

5 lines

python_modules/

dagster/

dagster/

core/

definitions/

1 line

Diff 38524

docs/content/api/modules.json

Loading...

docs/content/api/searchindex.json

Loading...

docs/content/api/sections.json

Loading...

docs/content/concepts/partitions-schedules-sensors/sensors.mdx

Loading...

docs/sphinx/sections/api/apidocs/schedules-sensors.rst

Loading...

examples/docs_snippets/docs_snippets/concepts/partitions_schedules_sensors/sensors/sensors.py

Loading...

examples/docs_snippets/docs_snippets_tests/concepts_tests/partitions_schedules_sensors_tests/test_sensors.py

Loading...

python_modules/dagster/dagster/core/definitions/sensor.py

Loading...