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

feat: Add Flank version info to requests #2073

Merged
merged 7 commits into from
Jul 8, 2021

Conversation

adamfilipow92
Copy link
Contributor

Fixes #1925

Test Plan

How do we know the code works?

In user agent Flank, sends version and revision.

This change adds version and revision to user agent by setApplicationName method. Let me know if it's fine. We can also add a new header client_info by using setTestingRequestInitializer and setToolResultsRequestInitializer.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2021

Timestamp: 2021-07-08 08:56:56
Buildscan url for ubuntu-workflow run 1011004148
https://gradle.com/s/gd4qz3duzpdlm

Copy link
Contributor

@bootstraponline bootstraponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, this isn't the right approach.

application name should stay the same. otherwise google is unable to pull stats on the flank client.

version info should be sent using client info details (as mentioned on the ticket)

@adamfilipow92
Copy link
Contributor Author

ah, this isn't the right approach.

application name should stay the same. otherwise google is unable to pull stats on the flank client.

version info should be sent using client info details (as mentioned on the ticket)

Thanks for the feedback, here is a current client_info example:


   "name":"Flank",
   "client_info_details":[
      {
         "key":"Flank Version",
         "value":"local_snapshot"
      },
      {
         "key":"Flank Revision",
         "value":"97854618886695c5005f9bc8d4e9a69eb7e922ed"
      }
   ]

I hope now everything is fine.

Copy link
Contributor

@bootstraponline bootstraponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me!

let's have one other member of the team approve as well.

Copy link
Contributor

@bootstraponline bootstraponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return self._messages.TestMatrix(
        testSpecification=spec,
        environmentMatrix=environment_matrix,
        clientInfo=client_info,
        resultStorage=results,
        flakyTestAttempts=self._args.num_flaky_test_attempts or 0)

https://github.com/Flank/gcloud_cli/blob/55ba35ba9d631096f05f3c0bfa5cf1182134b47e/google-cloud-sdk/lib/googlecloudsdk/api_lib/firebase/test/ios/matrix_creator.py#L177

https://github.com/Flank/gcloud_cli/blob/55ba35ba9d631096f05f3c0bfa5cf1182134b47e/google-cloud-sdk/lib/googlecloudsdk/api_lib/firebase/test/matrix_creator_common.py#L25

I looked at the code in gcloud and we're suppose to set clientInfo on the TestMatrix object, not via a testing request initializer.

image

@adamfilipow92
Copy link
Contributor Author

```python
return self._messages.TestMatrix(
        testSpecification=spec,
        environmentMatrix=environment_matrix,
        clientInfo=client_info,
        resultStorage=results,
        flakyTestAttempts=self._args.num_flaky_test_attempts or 0)

https://github.com/Flank/gcloud_cli/blob/55ba35ba9d631096f05f3c0bfa5cf1182134b47e/google-cloud-sdk/lib/googlecloudsdk/api_lib/firebase/test/ios/matrix_creator.py#L177
https://github.com/Flank/gcloud_cli/blob/55ba35ba9d631096f05f3c0bfa5cf1182134b47e/google-cloud-sdk/lib/googlecloudsdk/api_lib/firebase/test/matrix_creator_common.py#L25
I looked at the code in gcloud and we're suppose to set clientInfo on the TestMatrix object, not via a testing request initializer.

Right, thanks! I changed my implementation to using client info on the TestMatrix.

@pawelpasterz pawelpasterz enabled auto-merge (squash) July 8, 2021 09:24
@pawelpasterz pawelpasterz merged commit 5dadae6 into master Jul 8, 2021
@pawelpasterz pawelpasterz deleted the 1925-add-flank-version-to-client-info branch July 8, 2021 15:05
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Flank version info to client_info in the request
5 participants