search

kubernetes-retired / spartakus

[EOL] Anonymous Usage Collector
Apache License 2.0
74 stars 30 forks source link

Inquiry: Intention of Timestamp structure #29

Closed brianredbeard closed 5 years ago

brianredbeard commented 6 years ago

In pkg/volunteer/volunteer.go the Timestamp collected is being cast to a string

https://github.com/kubernetes-incubator/spartakus/blob/633430c4022c62dc1046682c7f3d3976eb783c60/pkg/volunteer/volunteer.go#L123

This is similarly defined in the schema pkg/database/bigquery.schema.json:

https://github.com/kubernetes-incubator/spartakus/blob/633430c4022c62dc1046682c7f3d3976eb783c60/pkg/database/bigquery.schema.json#L7-L11

As the service is primarily defined to utilize BigQuery as a datastore it seems odd to utilize a string versus a TIMESTAMP (or even an INTEGER since BigQuery stores this as int64).

Was the use of String an intentional design decision? What are the ramifications of changing this to a field of type Timestamp?

thockin commented 6 years ago

It was not intentional. We can probably do it compatibly - add a new field, have the server prefer the new field, and IFF the old field is needed, parse it back to a timestamp?

On Wed, Nov 1, 2017 at 5:54 PM, redbeard notifications@github.com wrote:

In pkg/volunteer/volunteer.go the Timestamp collected is being cast to a string

https://github.com/kubernetes-incubator/spartakus/blob/ 633430c4022c62dc1046682c7f3d3976eb783c60/pkg/volunteer/volunteer.go#L123

This is similarly defined in the schema pkg/database/bigquery.schema.json:

https://github.com/kubernetes-incubator/spartakus/blob/ 633430c4022c62dc1046682c7f3d3976eb783c60/pkg/database/ bigquery.schema.json#L7-L11

As the service is primarily defined to utilize BigQuery as a datastore it seems odd to utilize a string versus a TIMESTAMP (or even an INTEGER since BigQuery stores this as int64).

Was the use of String an intentional design decision? What are the ramifications of changing this to a field of type Timestamp?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kubernetes-incubator/spartakus/issues/29, or mute the thread https://github.com/notifications/unsubscribe-auth/AFVgVBCXSGWQvn5N_AMvmsvsaFObDCwIks5syRLegaJpZM4QPBmw .

brianredbeard commented 6 years ago

That should likely work. I first started exploring this a few weeks ago and took some notes on the BigQuery process. I hadn't considered the process of just migrating to a new field though (which really is probably the correct answer).

fejta-bot commented 5 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 5 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

brianredbeard commented 5 years ago

@thockin this has been quite a while. if i was to submit a PR to add the new field, would that work for you?

fejta-bot commented 5 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /close

k8s-ci-robot commented 5 years ago

@fejta-bot: Closing this issue.

In response to [this](https://github.com/kubernetes-incubator/spartakus/issues/29#issuecomment-504766241): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh with `/remove-lifecycle rotten`. > >Send feedback to sig-testing, kubernetes/test-infra and/or [fejta](https://github.com/fejta). >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.