GitHub Server action, to run DA on GitHub #740
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BryceStevenWilley
changed the title
This was referenced Aug 9, 2023
plocket
reviewed
Aug 13, 2023
plocket
reviewed
Aug 13, 2023
plocket
reviewed
Aug 13, 2023
|
Note: need to know what different errors look like in this process and document them (like missing env vars, missing config stuff, missing package dependencies, etc.) [Conclusion: For missing config/packages/general docker container issues, we'll just need to tell people that they'll have to look at the docker log or use tmate in order to troubleshoot the problem. Made https://github.com//issues/817] |
|
More notes in #753 (the PR made from that branch). The biggest issue I see as I work through this is that we run the github_server setup to get the interview on the server, but then we also run the regular setup and takedown, which will install the repo on the Playground. Do we want to double up that way? Is there a point to doing both? Where do we want to control whether those happen or not? Testing bugs so far: In an 08/14 test run:
Thoughts: Maybe we need to somehow break out into further actions to avoid duplication - an action for accepting input and creating env vars that happens before the other actions. It, or something else, would then need to manage calling the right follow-up actions. If we do that, the github server action can avoid the regular This seems complicated and would require conditional logic with another env var or input, which we were trying to avoid by splitting these actions into separate scripts (as we discussed). We should discuss which is worse - the duplication or the additional input/env var. (I'm reluctant to use another input as we're running into duplication problems with our current ones.) |
|
Seeing the state of things and the fact that we do still need to export / duplicate the env vars in both
If you're talking about v5 vs v4 duplication issues, this is v5 only. |
plocket
added
the
blocked
plocket
removed
the
blocked
plocket
force-pushed
the
start_server_take_2
branch
from
93d8cd1
to
2515806
Compare
Starting to look into starting Docassemble servers in Github actions, we needed to: * log-in to the server * create an API key, and save it * use said API key to install the package, so we have all of the dependencies This currently can install things, but can't properly keep track of the task_id for some reason? Need to look into it Also started the full bash script (which would become the Github actions script) for the process.
In progress! Much of the existing work came from https://github.com/BryceStevenWilley/SmolKilnPOC/blob/main/.github/workflows/smol.yml
if the var really does need to be saved in the config for storage in between runs or in between setup, test, and takedown.
This reverts commit 0507540.
Implement GitHub server workflow and action * Start new workflow file github_server.yml Create github_server.yml 🤞 Use branch with the new action in the new workflow Attempt to install package and test only two tests Double the waiting time Switch the action branch to the current branch Publish and use alkiln-startserver bin Install ALKiln with npm before using its darn script * Use full jhpyle da image. Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com> * Try to always print the final log, remove MAX default * Use custom config, output values, wait for docker to start Add tmate Separate wait into next step Remove conditional tmate Remove sudo Allow action to continue Lengthen sleep time to give us time with detached tmate Use input correctly (I hope...) Avoid running ALKiln in the composite action and... - Only start the server in the github_server composite action. - Create outputs in that action. Print them in the workflow that uses that composite action. - Use a while loop to detect whether docker is done starting. - Make a placeholder for a max docker start time input. Remove `needs`. LLM's aren't perfect. * Use INSTALL_METHOD input to control action.yml behavior Decrease initial server wait time, try to run tests Docker start time was much shorter than expected, so decreasing the amount of hard-coded time to wait before starting to test whether the server is loaded or not. Also, used a different variable for stopping the while loop so that race conditions don't prevent the success message from getting logged. Try to use var differently Try different syntax for var Try keeping env var. Can't use inputs just anywhere. Use the correct alkiln version for the command * Use relative paths for `uses:` to avoid editing val on every branch change Use branch ref in `uses`, hard code tags for now, tweak var names, clean up a bit Try different var syntax Try to use the full repo/branch ref as an env var Try a full path instead Go out two folders Try path code given by llm phind Try relative path again Remove the action.yml filename, just reference the dir Try to use var again Use relative path again, again going out 2 dirs Use tmate Use tmate better Just run tmate Remove the reference to the file itself * Use correct sign in step info for actual tests * Use `dainstall` and health check, save docker artifact Add tempt tmate to ALKiln test run to find interview on server Try different base url ending in data/questions/ Bump version for new base interview url Refine interview base url Log full interview url Our package name is different than our project name. Problem? Bump version for package url test Explore folders with tmate Looking for something like /usr/share/docassemble/local3.10/lib/python3.10/site-packages/docassemble/ALKiln/data/questions Match "ALKiln" folder name to package name as pip hasn't been able to install the package and we're theorizing that that's the issue. Change ALKilnTests to ALKiln, remove tmp dir, clean Add tmate step to experiment with dainstall Try to install using dainstall Isolate repo name Try new string substitution syntax Fix typo Remove debug log because its error is not worth solving Run tests again since installation seems to be working Actually activate tests Troubleshoot syntax that was working before Tweak indentation Allow server install address without js installing on server Bump feature version Switch back to hard-coded API key Set `result` in gh_server action Add AL requirement Add dangerous debug for debugging server action, clean up Test the failing test Log dangerous vars Bump for log Try a different password Try to use output email and password as env vars Log container name Create logs artifact, attempt 1 Typo Use absolute path for artifact Change startserver -> server_install file/function name Bump for file/function name change Start exploring url/healt_check output Try to disambiguate log of health check Try to get just the status code Tangent to try installing the package without cloning Add shell. There must be something global. Move reference to dir path for dainstall Add in a test that actually uses the package Use health_check response to check docker finished starting Conform to github action output guidelines Test docker install input Restoring defaults as the input test was successful Test all our tests * Make missing file error test more broad Make missing file error test more broad Bump for missing file test and console log * Remove `install_on_server` API request * Use dainstall to install repo on server and... use folder name to create interview address as the repo folder can have different name from repo name. Leave interviews installed on remote (non-github folders) and their installation process as it is. This is a combination of 11 commits. Change docassemble package folder back to ALKilnTests Add tmate to experiment with #egg Try to use inner folder name to get server location of interview Log more info about the chosen folders. Also try to use fewer tests Fail tag definition faster Bump dev to test with right ALKiln version. Pass tags as input. Add new ALKiln version Fix typo in function name Fix typo, use @server to install instead of @v# Fix requested version number typo Parse folder names differently * Switch install_type -> install_method, abstract config save/get * Run all ALKiln tests * Apply PR suggestions - Simplify getting docker logs now using our container name, though maybe this needs to use the var name - `exit 1` on docker error - Typo Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com> * Remove the var `ready`, comments and cleanup * Name the docker container (not getting output) Also temp reduce to one test for testing output * Handle no config value. Edit comments and clean. * Re-activate all github server tests, tweak logs and comments - Tweak logs and add comments - Re-activate all github server tests - More small clean up * Tweak folder name, improve console logs, date/time foldername, and clean up * Remove dangerous debug * Address docker log filename bash error with quotes and env var Address docker log filename bash error with quotes and env var Address docker log filename bash error with double quotes Address filename error (date?) with env var `TZ` instead of -u arg Restore one of the changes. Perhaps the bash is different in each place? Though they both use bash. Try double quotes Use env var for filename to allow download to find the right file Use env var for filename to allow download to find the right file --------- Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>
plocket
force-pushed
the
start_server_take_2
branch
from
a65b5fa
to
b5541a8
Compare
|
Docassemble core is giving the wrong error [and that's causing our tests to fail]. This is blocked till the next release when that should be fixed. [Fixed!] |
plocket
approved these changes
Oct 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Edit: Blocked by #760.In this PR, I have:
Reason for this PR
Get things running without external issues.
Closes #756
TODO: