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: export runtime config > .runtimeconfig.json #114

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

weilinzung
Copy link
Contributor

@weilinzung weilinzung commented Oct 22, 2021

Fixes #29

@weilinzung
Copy link
Contributor Author

@w9jds

@weilinzung weilinzung marked this pull request as draft October 22, 2021 14:29
@weilinzung
Copy link
Contributor Author

Need to confirm something first. to draft for now.

@w9jds
Copy link
Owner

w9jds commented Oct 22, 2021

Once you have confirmed yourself let me know, but his one looks fine.

@weilinzung
Copy link
Contributor Author

weilinzung commented Oct 22, 2021

@w9jds It is clearly no need with runtimeconfig.json when deploy from local with firebase deploy --only functions. Not sure why it keeps failing. Log here: #86 (comment)

Can we confirm the firebase-tools version that is used for this action?

@w9jds
Copy link
Owner

w9jds commented Oct 22, 2021

With the PR I just merged it is now v9.21.0

@weilinzung
Copy link
Contributor Author

@w9jds thanks. for the log I provided, do you have any insights? From my local testing with firebase-tools, everything is working fine without runtimeconfig.json.

@w9jds
Copy link
Owner

w9jds commented Oct 22, 2021

Not sure, but another PR that was just opened is going to allow you to set them as an environment variable, but it looks like since the code immediately does env.client_id it doesn't check to make sure that env is set which makes it sound like on live the config isn't set up.

@weilinzung
Copy link
Contributor Author

weilinzung commented Oct 22, 2021

Hmmm.
Also for the PR you mentioned that also has an other issue => #86 (comment)

It doesn't make sense to save as secrets because the config variables could be updated/unset from local and doing config:set with this action would overwrite the variable again.

@w9jds
Copy link
Owner

w9jds commented Oct 22, 2021

That should also work for paths, the firebase config cli command supports a path to a json file too.

@weilinzung
Copy link
Contributor Author

did a test with the latest version(6e669f5) with the latest firebase tool and still failed.

@w9jds
Copy link
Owner

w9jds commented Oct 22, 2021

Probably because you can't just export a file, and expect it to be accessible in other steps of a pipeline, that is why you have to use artifacts. From the change you just export to a file, you would need to somehow include it in an artifact for the next step to pull. Is accessing it in other steps your issue?

@weilinzung
Copy link
Contributor Author

weilinzung commented Oct 22, 2021

firebase deploy should already take care of the build. I can tell the build have no issue, just about the deploy part. Does matter we have to upload/download the artifact?

    "predeploy": [
      "npm --prefix \"$RESOURCE_DIR\" run lint",
      "npm --prefix \"$RESOURCE_DIR\" run build"
    ],
i  deploying functions
Running command: npm --prefix "$RESOURCE_DIR" run lint

> lint
> eslint "src/**/*"

Running command: npm --prefix "$RESOURCE_DIR" run build

> build
> tsc

✔  functions: Finished running predeploy script.
i  functions: ensuring required API cloudfunctions.googleapis.com is enabled...
i  functions: ensuring required API cloudbuild.googleapis.com is enabled...
✔  functions: required API cloudfunctions.googleapis.com is enabled
✔  functions: required API cloudbuild.googleapis.com is enabled

Error: Error occurred while parsing your function triggers.

TypeError: Cannot read property 'client_id' of undefined

@w9jds
Copy link
Owner

w9jds commented Oct 22, 2021

Yeah, but all you're doing is exporting config into a local file. Also, this change doesn't really address anything except for maybe your specific bug, which looks like a compile issue. Looking at this better, you are using typescript this is a TypeError which means compile error not firebase-tools related.

@weilinzung
Copy link
Contributor Author

weilinzung commented Oct 22, 2021

The thing is no issue at all when deploy from local.

People have same the issue here as well: https://stackoverflow.com/questions/69395116/firebase-functions-deployment-undefined-environment-datas-with-github-actions

@weilinzung
Copy link
Contributor Author

Close this PR for now. 100% we don;t need runtimeconfig.json for deploy. it is for sure not certain what is going on.

@weilinzung weilinzung closed this Oct 22, 2021
@weilinzung
Copy link
Contributor Author

@w9jds figured it out #86 (comment)

@weilinzung weilinzung reopened this Nov 18, 2021
@weilinzung
Copy link
Contributor Author

weilinzung commented Nov 18, 2021

@w9jds reopen this, it is useful when running emulators has runtimeconfig.

An example that need to get variables with functions.config(): https://firebase.google.com/docs/functions/config-env#access_environment_configuration_in_a_function

My test log:

Storing GCP_SA_KEY in /opt/gcp_key.json
Exporting GOOGLE_APPLICATION_CREDENTIALS=/opt/gcp_key.json
i  emulators: Starting emulators: functions, hosting, pubsub
⚠  functions: The following emulators are not running, calls to these services from the Functions emulator will affect production: auth, firestore, database, storage
✔  functions: Using node@14 from host.
⚠  functions: Your GOOGLE_APPLICATION_CREDENTIALS environment variable points to /opt/gcp_key.json. Non-emulated services will access production using these credentials. Be careful!
⚠  It appears you are running in a CI environment. You can avoid downloading the Pub/Sub Emulator repeatedly by caching the /github/home/.cache/firebase/emulators directory.
i  pubsub: downloading pubsub-emulator-0.1.0.zip...
i  pubsub: Pub/Sub Emulator logging to pubsub-debug.log
i  hosting[staging]: Serving hosting files from: dist/frontend
✔  hosting[staging]: Local server: http://localhost:5000
i  functions: Watching "/github/workspace/projects" for Cloud Functions...
⚠  It looks like you're trying to access functions.config().name but there is no value there. You can learn more about setting up config here: https://firebase.google.com/docs/functions/local-emulator
⚠  TypeError: Cannot read property 'client_id' of undefined
    at Object.<anonymous> (/github/workspace/projects/backend/lib/src/conversion-handler/utils.js:27:41)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:93:18)
    at Object.<anonymous> (/github/workspace/projects/backend/lib/src/conversion-handler/conversion-adjuster.js:8:17)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
⚠  We were unable to load your functions code. (see above)
   - It appears your code is written in Typescript, which must be compiled before emulation.
   - You may be able to run "npm run build" in your functions directory to resolve this.

@weilinzung weilinzung marked this pull request as ready for review November 18, 2021 00:32
@weilinzung
Copy link
Contributor Author

@w9jds any feedbacks?

@w9jds
Copy link
Owner

w9jds commented Nov 24, 2021

Have you tested this build with that particular test case to see if it actually resolves the issue?

@weilinzung
Copy link
Contributor Author

weilinzung commented Nov 24, 2021

@w9jds would you mind to help, where/how can I build this PR for testing from this action?

What I test so far is adding the runtime config to my branch and testing with this action, everything is good as long as the runtimeconfig is there.

@w9jds
Copy link
Owner

w9jds commented Nov 26, 2021

Ideally the best way to test is to create a branch fork or branch and then point your job to use the new version (not the docker image) then run the job you are trying to fix. If it does what you expected then the build is good to go. I don't have something that I could use reliably for what you are trying to solve.

@aliechti
Copy link

Is there still something going on here?

I need this feature to deploy the functions after merge. Otherwise it fails with:

Error: Error occurred while parsing your function triggers.

TypeError: Cannot read property 'app_id' of undefined

@w9jds
Copy link
Owner

w9jds commented May 14, 2022

If you want to test this @weilinzung I have setup a new deployment system for this action. What we can do it is resolve conflicts, then get this merged into master. The master image will deployed to docker hub and you can point to the master version and we should be able to run this in an actual action flow to make sure just exporting the file to local will be enough to share it between steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fetching config variables when deploying Firebase Functions
3 participants