-
Notifications
You must be signed in to change notification settings - Fork 200
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
base: master
Are you sure you want to change the base?
Conversation
Need to confirm something first. to draft for now. |
Once you have confirmed yourself let me know, but his one looks fine. |
@w9jds It is clearly no need with runtimeconfig.json when deploy from local with Can we confirm the firebase-tools version that is used for this action? |
With the PR I just merged it is now v9.21.0 |
@w9jds thanks. for the log I provided, do you have any insights? From my local testing with |
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 |
Hmmm.
|
That should also work for paths, the firebase config cli command supports a path to a json file too. |
did a test with the latest version(6e669f5) with the latest firebase tool and still failed. |
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? |
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?
|
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 |
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 |
Close this PR for now. 100% we don;t need runtimeconfig.json for deploy. it is for sure not certain what is going on. |
@w9jds figured it out #86 (comment) |
@w9jds reopen this, it is useful when running emulators has runtimeconfig. An example that need to get variables with My test log:
|
@w9jds any feedbacks? |
Have you tested this build with that particular test case to see if it actually resolves the issue? |
@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. |
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. |
Is there still something going on here? I need this feature to deploy the functions after merge. Otherwise it fails with:
|
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. |
Fixes #29