Skip to content

Use built-in cmp python function in comparing Datastore paths#3155

Closed
vikkyrk wants to merge 1 commit intoapache:masterfrom
vikkyrk:ds_py
Closed

Use built-in cmp python function in comparing Datastore paths#3155
vikkyrk wants to merge 1 commit intoapache:masterfrom
vikkyrk:ds_py

Conversation

@vikkyrk
Copy link
Contributor

@vikkyrk vikkyrk commented May 15, 2017

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify.
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@vikkyrk
Copy link
Contributor Author

vikkyrk commented May 15, 2017

R: @aaltay

@aaltay
Copy link
Member

aaltay commented May 15, 2017

LGTM, I can merge after tests.

Is there a unit test for compare_path ?

@vikkyrk
Copy link
Contributor Author

vikkyrk commented May 16, 2017

Yes they are here, https://github.com/apache/beam/blob/master/sdks/python/apache_beam/io/gcp/datastore/v1/helper_test.py. The problem is that query_splitter isn't being tested for both name and id. Fixed that.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b3124b1 on vikkyrk:ds_py into ** on apache:master**.

@aaltay
Copy link
Member

aaltay commented May 16, 2017

Thank you, LGTM. (I can merge after new tests.)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.222% when pulling b3124b1 on vikkyrk:ds_py into 69522fe on apache:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants