Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@ricardoamador
Copy link
Contributor

@ricardoamador ricardoamador commented Aug 3, 2022

Description

Save logs to staging for firebase tasks.

Issues

This addresses and OKR for this quarter for security improvements.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@keyonghan
Copy link
Contributor

@dnfield Are we still running firebase test lab test in engine CI?

@dnfield
Copy link
Contributor

dnfield commented Aug 4, 2022

We run firebase test lab but we don't use Cirrus at all..

This script gets invoked from https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/engine/engine.py#547

@keyonghan
Copy link
Contributor

We run firebase test lab but we don't use Cirrus at all..

This script gets invoked from https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/engine/engine.py#547

Got it. For this case, we will need to update the bucket then.

For context: we are moving the log location from flutter_firebase_testlab bucket to a new bucket flutter_firebase_testlab_staging. No breakage is expected, but please let us know if you see any issues after.

Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM

@keyonghan keyonghan changed the title Cirrus firebase logs staging Move firebase test lab logs to a staging bucket Aug 4, 2022
@keyonghan
Copy link
Contributor

Updated title and body to remove cirrus keywords.

import sys

BUCKET = 'gs://flutter_firebase_testlab'
BUCKET = 'gs://flutter_firebase_testlab_staging'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should this just be an env variable coming from LUCI?

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM. Let's put the project as an env as well. /cc @ricardoamador

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On it.

@ricardoamador
Copy link
Contributor Author

@keyonghan @dnfield to circle back the bucket and project have been updated in the firebase_testlab.py script in the engine repo. Do we need to change the bucket in the invoking recipe script? That one is using flutter_infra_release.

@keyonghan
Copy link
Contributor

@keyonghan @dnfield to circle back the bucket and project have been updated in the firebase_testlab.py script in the engine repo. Do we need to change the bucket in the invoking recipe script? That one is using flutter_infra_release.

After supporting env here, we need to update recipes to setup these two variable values there.

@keyonghan
Copy link
Contributor

We should hold this until the recipes env support.

Comment on lines 14 to 15
BUCKET = os.environ['STORAGE_BUCKET']
PROJECT = os.environ['GCP_PROJECT']
Copy link
Contributor

Choose a reason for hiding this comment

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

These should either get defaulted to something sensible or there should be an error thrown if they're not set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@ricardoamador
Copy link
Contributor Author

The Linux Android AOT Engine test will fail until the recipes change has been merged.
https://flutter-review.googlesource.com/c/recipes/+/32381

@ricardoamador ricardoamador merged commit 1ff8b17 into flutter:main Aug 4, 2022
@ricardoamador ricardoamador deleted the cirrus_firebase_logs_staging branch August 4, 2022 23:52
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 5, 2022
betrevisan pushed a commit to betrevisan/engine that referenced this pull request Aug 5, 2022
* Updated the bucket for firebase testing to the staging project.

* updated the project.

* Adding environment variables for gcp project and storage bucket.

* Updated to exit if environment variables were not provided.
emilyabest pushed a commit to emilyabest/engine that referenced this pull request Aug 12, 2022
* Updated the bucket for firebase testing to the staging project.

* updated the project.

* Adding environment variables for gcp project and storage bucket.

* Updated to exit if environment variables were not provided.
muditatandon pushed a commit to muditatandon/engine that referenced this pull request Aug 19, 2022
* Updated the bucket for firebase testing to the staging project.

* updated the project.

* Adding environment variables for gcp project and storage bucket.

* Updated to exit if environment variables were not provided.
muditatandon pushed a commit to muditatandon/engine that referenced this pull request Aug 19, 2022
* Updated the bucket for firebase testing to the staging project.

* updated the project.

* Adding environment variables for gcp project and storage bucket.

* Updated to exit if environment variables were not provided.
muditatandon pushed a commit to muditatandon/engine that referenced this pull request Aug 19, 2022
* Updated the bucket for firebase testing to the staging project.

* updated the project.

* Adding environment variables for gcp project and storage bucket.

* Updated to exit if environment variables were not provided.
ricardoamador added a commit to ricardoamador/engine that referenced this pull request Aug 19, 2022
* Updated the bucket for firebase testing to the staging project.

* updated the project.

* Adding environment variables for gcp project and storage bucket.

* Updated to exit if environment variables were not provided.
muditatandon pushed a commit that referenced this pull request Aug 19, 2022
* Updated the bucket for firebase testing to the staging project.

* updated the project.

* Adding environment variables for gcp project and storage bucket.

* Updated to exit if environment variables were not provided.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants