Skip to content

Rename DatasetDagRef to DagScheduleDatasetReference#25920

Merged
dstandish merged 3 commits intoapache:mainfrom
astronomer:dag-dataset-consumer-rename
Aug 28, 2022
Merged

Rename DatasetDagRef to DagScheduleDatasetReference#25920
dstandish merged 3 commits intoapache:mainfrom
astronomer:dag-dataset-consumer-rename

Conversation

@dstandish
Copy link
Contributor

Makes more sense to name with <thing doing the referencing>_<thing that is referenced>.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:Scheduler including HA (high availability) scheduler area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Aug 23, 2022
@dstandish dstandish changed the title Rename DatasetdagRef to DagDatasetConsumer Rename DatasetDagRef to DagDatasetConsumer Aug 23, 2022
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as other PR, wdyt about DatasetConsumer or DatasetConsumerDag instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 DatasetConsumerDag

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 DatasetConsumerDag TOO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, i guess the people have spoken

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll just mention though... the reason i thought it makes sense to put Dag first, is cus it's the reference from dag to dataset.

so if we didn't have to specify type of reference (i.e. consumer vs producer) then it would be a pretty easy choice i think.... we'd just call it dag_dataset since it's just a mapping from dag to dataset.

and so i figured, just add a suffix indicating type: dag_dataset_consumer

but, i guess another way to think about it is, not so much as a mapping table, but more as an entity unto itself, e.g. a DatasetConsumer entity of type Dag.... which... i guess is how y'all are thinking about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the other one, maybe DatasetConsumingDag instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually had other thought please see thread #25919 (comment)

@dstandish dstandish changed the title Rename DatasetDagRef to DagDatasetConsumer Rename DatasetDagRef to DatasetConsumerDag Aug 24, 2022
@dstandish dstandish force-pushed the dag-dataset-consumer-rename branch from 591c954 to 2280431 Compare August 24, 2022 23:27
@dstandish dstandish changed the title Rename DatasetDagRef to DatasetConsumerDag WIP Rename DatasetDagRef to DatasetConsumerDag Aug 26, 2022
@dstandish dstandish changed the title WIP Rename DatasetDagRef to DatasetConsumerDag Rename DatasetDagRef to DagScheduleDatasetReference Aug 26, 2022
@dstandish dstandish force-pushed the dag-dataset-consumer-rename branch from 2280431 to 4d45006 Compare August 26, 2022 21:43
Makes more sense to name with `<thing doing the referencing>_<thing that is referenced>`.
@dstandish dstandish force-pushed the dag-dataset-consumer-rename branch from 4d45006 to 6173db4 Compare August 28, 2022 05:10
@dstandish dstandish merged commit 5f333b4 into apache:main Aug 28, 2022
@dstandish dstandish deleted the dag-dataset-consumer-rename branch August 28, 2022 06:02
@jedcunningham jedcunningham added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) AIP-48 labels Sep 12, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.0 milestone Sep 14, 2022
@eladkal eladkal added the area:data-aware-scheduling assets, datasets, AIP-48 label Mar 25, 2025
@eladkal eladkal removed the AIP-48 label Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:data-aware-scheduling assets, datasets, AIP-48 area:Scheduler including HA (high availability) scheduler area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants