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

Kill off 'DEFAULT_TOPIC_NAME'. #2195

Merged
merged 1 commit into from
Aug 25, 2016
Merged

Kill off 'DEFAULT_TOPIC_NAME'. #2195

merged 1 commit into from
Aug 25, 2016

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 25, 2016

The hypothesis is that we are seeing flakiness due to rapid creation / deletion cycles on topics with the shared name.

Towards #2163, #2192.

The hypothesis is that we are seeing flakiness due to rapid creation /
deletion cycles on topics with the shared name.

Towards #2163, #2192.
@tseaver tseaver added api: pubsub Issues related to the Pub/Sub API. flaky labels Aug 25, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 25, 2016
@tseaver
Copy link
Contributor Author

tseaver commented Aug 25, 2016

@dhermes, @daspecster I'm tempted to kill off the Travis for the PR and merge immediately: the system tests pass with this change on my box, and tox -e lint is happy. Since the only point of it is to try to fix post-PR builds, I'd like to know whether it works. :)

@dhermes
Copy link
Contributor

dhermes commented Aug 25, 2016

The resource ID has per-second granularity on Travis. You probably want to go lower by a factor of 100. Add an optional argument to multiply seconds in time.time() before casting to an int?

@dhermes
Copy link
Contributor

dhermes commented Aug 25, 2016

@tseaver I urge you to resist the temptation. Travis won't run your job for hours so it's not worth subverting the review process

@dhermes
Copy link
Contributor

dhermes commented Aug 25, 2016

I just read the definition of "subvert". A very dramatic word

@tseaver
Copy link
Contributor Author

tseaver commented Aug 25, 2016

The resource ID has per-second granularity on Travis. You probably want to go lower by a factor of 100. Add an optional argument to multiply seconds in time.time() before casting to an int?

We're already mangling in the TRAVIS_BUILD_ID into the unique_resource_id return value on Travis: are you worried we will be running two jobs with the same ID? The theory here is that reusing a topic ID within one run is the source of the flakiness we are seeing in #2163 and #2192.

Travis won't run your job for hours so it's not worth subverting the review process

That means it will be double that number of hours before we know whether it works.

@dhermes
Copy link
Contributor

dhermes commented Aug 25, 2016

Sorry I didn't realize your prefixes were unique, so it's moot.

LGTM to merge (no need to wait on Travis if you've run all the tox environmentsvthst Travis will)

@dhermes
Copy link
Contributor

dhermes commented Aug 25, 2016

*environments that (typing on phone, sorry)

@tseaver tseaver merged commit fd0e6bf into googleapis:master Aug 25, 2016
@tseaver tseaver deleted the 2163-2192-pubsub-system-tests-unavailable branch August 25, 2016 14:48
@tseaver
Copy link
Contributor Author

tseaver commented Aug 25, 2016

Fingers crossed

@tseaver
Copy link
Contributor Author

tseaver commented Aug 25, 2016

W00t! the remaining failures are due to #2176.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants