Config file Auto-generation tool

[ykaravas]

This pull request closes #136

Added configuration file autogeneration tool. I think there is still a lot that could be done here and there is potential for a lot more customization of tests/runs. The conversation is ongoing, however, this is a good spot to review and maybe even merge as it has basic functionality and is stable.

Moved configuration files around repo to their own directory, opencbdc-tx/config, for organizational purposes. Updated related scripts and docker yml files with new location of configs. This config dir houses an integration dir which holds integration test configs, unit dir, which holds unit test configs, a general dir, which holds the configs which were formerly in the project root dir and a tools dir which holds the config generator.

Created simple unit test for config generation as well which could be expanded a bit.

[HalosGhost]

This looks like a solid first crack. I have a few requests inline and a couple of questions.

[ykaravas]

@HalosGhost I believe i have addressed all your addressable comments made in this PR. The ones that were very straightforward i have marked "DONE". Others that i either have questions about or have implemented but should probably be reviewed again i have marked as "NEEDS FOLLOW UP".

I have consolidated all my changes in one commit but have not squashed that commit into the original commit for this PR yet so that you can more easily view the diff.

[ykaravas]

@HalosGhost This is just a general question. I am not sure where to post something like that. Would it be Zulip? Anyways, my question is, do you see, in your environment, any integration tests sporadically failing? I never used to see that occurring but recently, atomizer double_spend or atomizer invalid transaction sporadically fail. This occurs on all branches. I had not seen this until this week. I am just curious if this is a common occurrence or not.

[HalosGhost]

@HalosGhost This is just a general question. I am not sure where to post something like that. Would it be Zulip?

Zulip is a great catch-all for if you're not sure where to post something. However, there is a feedback/question tag for issues if it feels technically-relevant enough (as this does to me).

[…] do you see, in your environment, any integration tests sporadically failing? I never used to see that occurring but recently, atomizer double_spend or atomizer invalid transaction sporadically fail. This occurs on all branches. I had not seen this until this week. I am just curious if this is a common occurrence or not.

We are aware of several cases where they are more likely to fail now than they used to be (cf. #119). However, even if this feels more pronounced now, it's an extension of the fact that the test suite is a bit fragile at the moment (e.g., relying on sleep() calls hoping that the task has finished rather than a less racy solution).

I've also seen this a little more frequently in the GitHub CI of-late, but I think the real solution is a general overhaul of how some of the test suite is written/architected rather than any change (e.g., increasing the sleep times).

[ykaravas]

@HalosGhost Hey Sam, are there any blockers on merging this? I am beginning work on a subsequent branch to create a test to use these files as well as further organize what i have done.

[HalosGhost]

There is a conflict to resolve, but it should be minor. Also, I must apologize for the delay in-review. My attention has been elsewhere; I will try to take a look at it this weekend!

Also, please feel free to stack PRs if you're going to work on a follow-up branch (just base the new branch on this one, and reference this PR in the PR text to make clear this needs to be merged first).

[ykaravas]

@HalosGhost Completely understand about the delays! I am sure you have a very full plate so no worries! I just wanted to make sure you werent waiting on me thats all. I did not notice the conflict. I will resolve that soon. I have branched off this for my continuation yes. Thanks for the tips!

[HalosGhost]

Sorry this review took quite a while, but I tried to be quite thorough. I do think there are a few things that need to change before merge; most of them are hopefully simplifications and not making anything more complex.

[HalosGhost]

The specific error is here:

cp: cannot stat '/home/runner/work/opencbdc-tx/opencbdc-tx/scripts/../config/tools/*.tmpl': No such file or directory

In particular, it looks like the culprit is this line. Looking at the changed-files list in the PR, <repo_root>/config/tools/*.tmpl does not appear to exist.

As a side-note: cp <wildcard-which-matches-multiple-files> target is rather dangerous (this predated your PR), but we should probably switch to using the find… construction at line 167

[HalosGhost]

@ykaravas looks like you have it handled. utACK. Will test locally ASAP (heads-up that the next few days are rather busy on my end so it might be a little while before I can test fully).

[HalosGhost]

@ykaravas I've started taking another look at this and I'm getting stumped earlier in the process. Namely, the test suite seems to hang indefinitely for me on the config-test validation. I reran the CI to see if the same issue would occur, but it ran fine. The only change I made was to rebase on trunk; can you do a git pull --rebase upstream trunk (assumes you have mit-dci/opencbdc-tx setup as a remote named upstream in your local clone), and see if you run into issues?

As a side-note: please put two returns between your commit header and the body; right now, the full commit message is in the header (which is unreadable).

[ykaravas]

@HalosGhost That is odd. I have not been running into any hanging issues. I will test more on my end to see if i do. I have been rebasing all along. I will do that again in case it needs it but i think it is up to date.

And what do you mean about two returns between commit header and body? I usually run the "git commit -s -m "commit message" command. Is there some other way i should do it?

[HalosGhost]

@ykaravas I'm so sorry, I accidentally edited your comment rather than replying! I've copied the result of my edit below for clarity, and I returned your comment to its original state. My apologies!

And what do you mean about two returns between commit header and body? I usually run the "git commit -s -m "commit message" command. Is there some other way i should do it?

That makes sense. It's a little more typical to omit -m (which will then open a full editor (by default, usually vim, but this is configurable)). In doing so, the first line you enter in the editor corresponds to what you'd pass to -m (by convention, kept to <50 characters); hitting return twice (leaving an empty line between the initial line and the rest of the message) then starts the body of the message.

If you look through the commit history, you can see that some committers will expand quite a bit in the message body about why a change was done, or the general motivation. But, when keeping the header to <50 characters, you almost have to because that header is going to be the simplest summary of the change.

[ykaravas]

@HalosGhost Have you had a chance to test this out yet?

[HalosGhost]

@ykaravas I'm still a little unsure of what's happening. But, the test suite is still hanging indefinitely for me. I'm going to stepwise debug it here in the next couple of days unless you have a thought of what might be going wrong.

[ykaravas]

@HalosGhost Hey Sam. I am not sure what could be going on as i cannot recreate that issue. It seems that the unit test is passing on the server even though. So it doesn't seem to be hanging there either as far as i can tell, nor does it on my system. I am not sure how to go about debugging it since i cannot recreate it though. Any suggestions?

[HalosGhost]

@ykaravas nothing immediately comes to mind. Stepwise debugging on my side is the next reasonable step; sorry I've not been able to do so earlier!

[HalosGhost]

Ah, I've found what's wrong. It's a bit of fragility, but I'm not actually sure of a pleasant way to fix it. My clone is not named opencbdc-tx. As a result, the routine to find the root-directory of the repo fails because the parent-directory it's looking for doesn't exist. Thus, that loop looking to find the repo-root runs infinitely.

I'll disregard that for now and plan to use my local CI emulation until/unless we figure out a less-fragile method of detecting the repo root.

[HalosGhost]

Note: the way you do this in-shell is actually quite simple:

$ cd "$(git rev-parse --show-toplevel)"

[HalosGhost]

Confirmed. I've changed my local clone's containing name to make testing easier. ☺

[HalosGhost]

@ykaravas I think we should change one more thing. Right now, the config-generator finishes by just saying SUCCESS if all went well. It doesn't tell you the name of the config generated nor where that generated config is. I think a really easy fix to this is to instead output the generated configuration on stdout (note that SUCCESS as well as the other status messages like where the build directory has been determined to be should not be on stdout (which is where they're printed now), but rather stderr if we make this change). That way, users can simply redirect the output of the tool to their intended target (naming and location is then up to the user). This might also mean that we can avoid trying to find the project root or build directory (though I'll defer to you on that one).

[HalosGhost]

As I mentioned above, I think there are two choices for the final change:

1. Move all diagnostic/status messages to stderr, and output the generated config on stdout (instructing users to redirect the output to their intended name/location)
2. Output the final path of the generated config along with the SUCCESS message.

I'm personally a fan of the first, but the second is definitely less work and I'd find it acceptable. The only thing that might strongly tip me in-favor of the first option is if it removes the need to locate the root/build directories. If so, then it can remove a ton of fragility that we'd be better off without.