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.

Sign up for GitHub

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

Jump to bottom

Grpcio pre system tests #5304

Merged
merged 1 commit into from
May 16, 2018
Merged

Grpcio pre system tests #5304

merged 1 commit into from
May 16, 2018

Conversation

crwilcox
Copy link
Contributor

@crwilcox crwilcox commented May 7, 2018

No description provided.

@crwilcox
Modify system tests to use prerelease versions of grpcio
4e5e350 
Update documentation to have correct entity string repr
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 7, 2018
@crwilcox
Copy link
Contributor Author

crwilcox commented May 7, 2018

Logging and BigTable will likely fail. This is due to #5303 (Logging) and #5302 (BigTable)

tswast
tswast reviewed May 7, 2018
View reviewed changes
bigquery/nox.py
@@ -93,6 +93,9 @@ def system(session, py):
# Set the virtualenv dirname.
session.virtualenv_dirname = 'sys-' + py

# Use pre-release gRPC for system tests.
session.install('--pre', 'grpcio')

This comment was marked as spam.

Sign in to view

tswast
tswast reviewed May 7, 2018
View reviewed changes
bigquery_datatransfer/nox.py
@@ -86,6 +86,9 @@ def system(session, py):
# Set the virtualenv dirname.
session.virtualenv_dirname = 'sys-' + py

# Use pre-release gRPC for system tests.
session.install('--pre', 'grpcio')

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor

tseaver commented May 8, 2018

@crwilcox How long do we expect to need the pre-release version of grpcio? Also, it would be useful to link the issue(s) here that the new pre-release solves, to give us scope about whether / when to revert.

@tswast
Copy link
Contributor

tswast commented May 8, 2018

@tseaver I assumed this was proactive on our part so that we could catch errors in gRPC that cause issues in the client libs before the release. I agree it would be good to get some clarification as to why we are adding this (ideally as comments in the code).

@tseaver
Copy link
Contributor

tseaver commented May 8, 2018

@tswast I'm actually opposed to doing our normal CI against pre-releases of dependencies: we need more stability in our everyday runs, not less. It would be fine if we had an additional (maybe nightly?) "smoke test" for such things.

@tswast
Copy link
Contributor

tswast commented May 8, 2018

I'd be on board with splitting risky / flakey / slow & only moderate value system tests into a nightly build.

@theacodes
Copy link
Contributor

That sounds lovely, unfortunately, our current CI is working against us here. Let's defer that until we move to Kokoro.

@crwilcox crwilcox merged commit 5644f15 into master May 16, 2018
@tseaver
Copy link
Contributor

tseaver commented May 16, 2018

@crwilcox You merged this one without getting consensus about the tradeoffs of using prerelease dependencies. I will ask again: Is there a specifc issue addressed by a particular grpcio prerlease that you are trying to fix? If not, then I think we should reconsider making our everyday CI more fragile.

@tseaver
Copy link
Contributor

tseaver commented May 16, 2018

Note that #5333 (addressing https://circleci.com/gh/GoogleCloudPlatform/google-cloud-python/6613) is the kind of flakiness I'm talking about: we are now in a mode where we can't run against both the released and the unreleased grpcio libraries, which is not a Good Thing(TM).

@tseaver tseaver mentioned this pull request May 17, 2018
Merged
@tseaver tseaver deleted the grpcio-pre-system-tests branch June 13, 2018 17:51
atulep pushed a commit that referenced this pull request Apr 3, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
58da75e
atulep pushed a commit that referenced this pull request Apr 18, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
2a9da6d
parthea pushed a commit that referenced this pull request Jul 6, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
8c2aaa3
parthea pushed a commit that referenced this pull request Sep 22, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
40eac27
parthea pushed a commit that referenced this pull request Sep 22, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
a9223b2
parthea pushed a commit that referenced this pull request Sep 22, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
aed3c01
parthea pushed a commit that referenced this pull request Sep 22, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
4f38d22
parthea pushed a commit that referenced this pull request Oct 21, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
b781324
parthea pushed a commit that referenced this pull request Oct 21, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
d9bba84
parthea pushed a commit that referenced this pull request Oct 21, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
45f1af6
parthea pushed a commit that referenced this pull request Oct 22, 2023
@crwilcox
Modify system tests to use prerelease versions of grpcio (#5304)
523296d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tswast tswast tswast left review comments

@dhermes dhermes Awaiting requested review from dhermes

@lukesneeringer lukesneeringer Awaiting requested review from lukesneeringer

@tseaver tseaver Awaiting requested review from tseaver

@waprin waprin Awaiting requested review from waprin

Assignees
No one assigned
Labels
api: core cla: yes This human has signed the Contributor License Agreement. testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@crwilcox @tseaver @tswast @theacodes @googlebot