Remove `Property._CREATION_COUNTER` from `ndb`

[dhermes]

Stepping into the original source, it's clear it was not used for anything:

$ cd datastore-ndb-python/
$ git grep -n '_creation_counter'
RELEASE_NOTES:112:- Issue 175: Add _creation_counter.
ndb/model.py:888:  __creation_counter_global = 0
ndb/model.py:942:    Property.__creation_counter_global += 1
ndb/model.py:943:    self._creation_counter = Property.__creation_counter_global
ndb/model_test.py:1230:    self.assertTrue(MyModel.foo._creation_counter <
ndb/model_test.py:1231:                    MyModel.bar._creation_counter <
ndb/model_test.py:1232:                    MyModel.baz._creation_counter)

It seems the stated goal in GoogleCloudPlatform/datastore-ndb-python#175 (when this was introduced) was never implemented.

@tseaver @chrisrossi WDYT?

@theacodes Do we have leeway to get rid of such a feature?

[chrisrossi]

I don't think it was meant to be used in the package. It seems the intent was to provide a means to allow applications using NDB to sort properties using it. While that doesn't seem super useful to me, there's likely somebody somewhere using it who will holler if we don't port it.

[dhermes]

1. I somewhat agree with the sentiment
2. It'd be nice to find an example of such usage
3. I've already modified the name (__creation_counter_global -> _CREATION_COUNTER)
4. The names cls.__creation_counter_global and self._creation_counter are non-public, so it's kind of not "allowed" for users to depend on them

I still don't know what the right move is here, just offering up these remarks.

[chrisrossi]

Re: 4. Not true unfortunately. "All special Property attributes, even those considered 'public', have names starting with an underscore. This is because StructuredProperty uses the non-underscore attribute namespace to refer to nested Property names; this is essential for specifying queries on subproperties."

https://cloud.google.com/appengine/docs/standard/python/ndb/subclassprop

[dhermes]

Oy! I think that is just referring to attributes of the Property instances, e.g. _name, _indexed, etc.

[chrisrossi]

You're probably right. I googled for it and don't see any obvious cases of it being used on anything published publicly. Maybe leave it out and let 'em file an issue if it breaks their stuff.

[dhermes]

let 'em file an issue if it breaks their stuff.

I have a feeling there will be a lot of this.

Maybe leave it out

Do you want to send the PR to fix this issue?

[chrisrossi]

Do you want to send the PR to fix this issue?

How do you mean?

[dhermes]

@chrisrossi Sorry, I meant the stated goal of this issue, i.e. remove Property._CREATION_COUNTER from ndb

[crwilcox]

I think removing internal fields is acceptable. Though we should keep note of the ones we don't implement and consider carefully. With the age of NDB there are going to be plenty of these people have taken dependencies on. I can't find an instance of this particular internal field being used outside of the library so I am comfortable with this one getting removed.

[pj]

Not sure if it's relevant, but I've built tools that use fields like _name to do things like data snapshot generation and generate schemas for big query.