[Playground] Add config.g.dart+Fix gradle #25022
Conversation
| val lines = file.readLines() | ||
| val endOfSlice = lines.indexOfFirst { it.contains("Generated content below") } | ||
| if (endOfSlice != -1) { | ||
| val oldContent = lines.slice(0 until endOfSlice) | ||
| val flagDelete = file.delete() | ||
| if (!flagDelete) { | ||
| throw kotlin.RuntimeException("Deleting file failed") | ||
| } | ||
| val sb = kotlin.text.StringBuilder() | ||
| val lastLine = oldContent[oldContent.size - 1] | ||
| oldContent.forEach { | ||
| if (it == lastLine) { | ||
| sb.append(it) | ||
| } else { | ||
| sb.appendLine(it) | ||
| } | ||
| } | ||
| file.writeText(sb.toString()) | ||
| } |
There was a problem hiding this comment.
This Gradle task can be executed in these two cases:
- The user has just typed in the command to deploy the system for the first time after checking it out.
config.g.dartis as the version-controlled copy. It has the production URLs which developers normally use for their local runs. They are of no meaning to the deployment and must be overwritten. Here the string "Generated content below" is the flag that this script uses to detect thatconfig.g.darthas not been written with the deployment URLs. It then overwritesconfig.g.dartusingdns-nameproperty value. This property value must be provided. - The user is re-deploying the system from the same checked-out directory, so
config.g.dartalready contains the deployment URLs. The idea was to allow the user to re-run the task without specifyingdns-nameto use the same value as before. Sinceconfig.g.dartnow does not contains "Generated content below" line, it will not be overwritten. Handling this case is the only purpose of preserving the content.
If we must accept this fast, then this is the way to go because normally we will only run this once per check-out-and-deploy cycle. This is my guess because I am not an expert in Gradle.
But if we have time to clean this up, I suggest doing so, because:
- Seems like we do not support re-running the task. Since "Generated content below" will not be found in the overwritten file,
file.delete()will not be called, and soconfig.g.dartwill be appended with new values, which in Dart means duplicate definition of constants, so it will not build. - If we fix this, the very search for a content in the file is an unreliable flag for a need to overwrite it because the content may change for reasons outside of this script. I see the following as more reliable:
- Always overwrite the file if
dns-nameproperty is given. Always skip any writes ifdns-nameis not given. - Always require
dns-nameand throw if it is not provided.
I personally think that we should require all parameters be given when running the task. I can foresee other configurable options that are deployment-specific:
- Google Analytics counter ID.
- Debug flags.
While it is easy to add -Pname=value flags for them to a Gradle task and to write them into config.g.dart, it is increasingly hard to allow omitting any of them.
@damondouglas what do you suggest?
There was a problem hiding this comment.
@MakarkinSAkvelon / @alexeyinkin Do we really need a gradle task wrapper to generate the config.g.dart file? What would be the consequences if we do not automate its generation?
There was a problem hiding this comment.
I am not an expert in deployment. config.g.dart already generated with Gradle, and this PR just allows this file to pre-exist under some conditions. Given that we are tired of copying config.example.dart to config.g.dart manually, I think this is worthy short-term.
Mid-term I just want something clean and simple, idempotent with respect to command runs, and that will allow me any changes in the version-controlled copy. This currently is not idempotent and requires us to preserve a specific comment in the version-controlled copy.
Whether we should merge this first or go directly with the mid-term goal is not up to me because I do not know the priorities in DevOps.
I also think we also need a diagram of runs, like what starts what, because I am currently lost in this.
There was a problem hiding this comment.
In case we leave this file - config.g.dart - "as-is" Frontend will use the wrong Backend URLs and the playground will not work at all
damondouglas
left a comment
damondouglas
left a comment
There was a problem hiding this comment.
@MakarkinSAkvelon see question/comments.
| val lines = file.readLines() | ||
| val endOfSlice = lines.indexOfFirst { it.contains("Generated content below") } | ||
| if (endOfSlice != -1) { | ||
| val oldContent = lines.slice(0 until endOfSlice) | ||
| val flagDelete = file.delete() | ||
| if (!flagDelete) { | ||
| throw kotlin.RuntimeException("Deleting file failed") | ||
| } | ||
| val sb = kotlin.text.StringBuilder() | ||
| val lastLine = oldContent[oldContent.size - 1] | ||
| oldContent.forEach { | ||
| if (it == lastLine) { | ||
| sb.append(it) | ||
| } else { | ||
| sb.appendLine(it) | ||
| } | ||
| } | ||
| file.writeText(sb.toString()) | ||
| } |
There was a problem hiding this comment.
@MakarkinSAkvelon / @alexeyinkin Do we really need a gradle task wrapper to generate the config.g.dart file? What would be the consequences if we do not automate its generation?
|
config.g.dart is deleted here instead: |
4 participants
resolves #24200
This PR add config.g.dart file with standard volumes
Change build.gradle.kts file (changing values in config.g.dart file during execution)
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.