Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docstrings do not specify which fields are one-ofs #713

Closed
busunkim96 opened this issue Dec 16, 2020 · 0 comments · Fixed by #1030
Closed

Docstrings do not specify which fields are one-ofs #713

busunkim96 opened this issue Dec 16, 2020 · 0 comments · Fixed by #1030
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: docs Improvement to the documentation for an API.

Comments

@busunkim96
Copy link
Contributor

It is not currently possibly to tell that a field is part of a one-of by just looking at the docstring.

Example:

https://github.com/googleapis/python-dlp/blob/4f3148e93ec3dfc9395aa38a3afc62498500a055/google/cloud/dlp_v2/types/dlp.py#L3740-L3761

class Action(proto.Message):
    r"""A task to execute on the completion of a job.
    See https://cloud.google.com/dlp/docs/concepts-actions to learn
    more.
    Attributes:
        save_findings (~.dlp.Action.SaveFindings):
            Save resulting findings in a provided
            location.
        pub_sub (~.dlp.Action.PublishToPubSub):
            Publish a notification to a pubsub topic.
        publish_summary_to_cscc (~.dlp.Action.PublishSummaryToCscc):
            Publish summary to Cloud Security Command
            Center (Alpha).
        publish_findings_to_cloud_data_catalog (~.dlp.Action.PublishFindingsToCloudDataCatalog):
            Publish findings to Cloud Datahub.
        job_notification_emails (~.dlp.Action.JobNotificationEmails):
            Enable email notification for project owners
            and editors on job's completion/failure.
        publish_to_stackdriver (~.dlp.Action.PublishToStackdriver):
            Enable Stackdriver metric dlp.googleapis.com/finding_count.
    """

All of the above fields are part of a one-of 'action'.

https://github.com/googleapis/python-dlp/blob/4f3148e93ec3dfc9395aa38a3afc62498500a055/google/cloud/dlp_v2/types/dlp.py#L3837-L3862

    save_findings = proto.Field(
        proto.MESSAGE, number=1, oneof="action", message=SaveFindings,
    )


    pub_sub = proto.Field(
        proto.MESSAGE, number=2, oneof="action", message=PublishToPubSub,
    )


    publish_summary_to_cscc = proto.Field(
        proto.MESSAGE, number=3, oneof="action", message=PublishSummaryToCscc,
    )


    publish_findings_to_cloud_data_catalog = proto.Field(
        proto.MESSAGE,
        number=5,
        oneof="action",
        message=PublishFindingsToCloudDataCatalog,
    )


    job_notification_emails = proto.Field(
        proto.MESSAGE, number=8, oneof="action", message=JobNotificationEmails,
    )


    publish_to_stackdriver = proto.Field(
        proto.MESSAGE, number=9, oneof="action", message=PublishToStackdriver,
    )

This can lead to confusing behavior when the developer tries to (1) set multiple fields in the constructor or (2) sets multiple fields in multiple calls. In both cases only the last field is preserved.

Follow up to googleapis/proto-plus-python#170

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Dec 17, 2020
@software-dov software-dov added priority: p2 Moderately-important priority. Fix may not be included in next release. type: docs Improvement to the documentation for an API. and removed triage me I really want to be triaged. labels Dec 21, 2020
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Mar 22, 2021
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Jun 16, 2021
@busunkim96 busunkim96 self-assigned this Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants