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

Backchannels Moved to Framework Repos #881

Open
nodlesh opened this issue Oct 22, 2024 · 11 comments
Open

Backchannels Moved to Framework Repos #881

nodlesh opened this issue Oct 22, 2024 · 11 comments
Assignees

Comments

@nodlesh
Copy link
Contributor

nodlesh commented Oct 22, 2024

Backchannels are moved out of AATH repo.

Follow the pattern of the Aries-VCX backchannel

@ianco ianco self-assigned this Nov 26, 2024
@ianco
Copy link
Contributor

ianco commented Nov 26, 2024

In the VCX repo, the CI builds and publishes an image containing the VCX backchannel. The Dockerfile in this OATH repo (for this backchannel) just references the name of the publishes image.

For Aca-Py, we'll need to update the Aca-Py repo to contain all backchannel code, and to publish an image each time the main branch is updated in the repo.

(For local testing, one can build a local backchannel image and then update the Dockerfile in the OATH repo to point to the locally generated image).

@ianco
Copy link
Contributor

ianco commented Nov 29, 2024

Thoughts on the backchannel migration to aca-py, and a few questions. @swcurran @nodlesh @jamshale

For Aca-py, I suggest we support 3 builds for the backchannel:

  • main, built off the github main branch (existing)
  • latest, built from the latest aca-py release (existing)
  • local, built from the local source code (for developer testing prior to check-in)

(Question - There are 2 other supported builds - acapy-redis and dev-acacpy-main, which I'm not sure we actually need or use?)

(There is also remote, which is coded in the OATH manage script, but doesn't have a supported aca-py build, which I don't think we need to the aca-py repo side of things, so I'll leave that in the OATH repo scripts.)

In the aca-py repo, the build scripts will build a local image, same as the OATH script currently does. So to run tests locally, you can run the build scripts in the aca-py repo, and then just build and run the test harness in the OATH repo (no need to build the backchannels twice).

(Question - in the OATH ./manage build script, if you leave out the -a argument to build a specific agent, it will build all agents. I think this is incorrect and if -a isn't specified it should build the test harness only and no backchannels?)

(If you are running OATH without building any local images, you will need to run the local OATH build script, which will download the necessary published images.)

In the aca-py repo, building the backchannel images will be supported by local scripts, and publishing the images will be supported by github actions. The main image should be published each time the main branch is updated, and the latest image should be published each time a new version of aca-py is released.

(Question - is it better to publish a new main backchannel each time the main branch is updated, or is it sufficient to publish on a schedule, e.g. nightly?)

(Question - do we need to maintain a history of aca-py backchannels, so that new versions can be regression-tested against previous versions?)

@swcurran
Copy link
Contributor

Some thoughts in response:

  • I would hope that we don’t add builds to ACA-Py, since we already have the nightly and release builds. I don’t think we need more artifacts. Unless we eliminate the nightly, we don’t want to publish new artifacts on updating main.
  • I’m assuming that if you want to run the OATH tests, you would do so from the OATH repo — not from ACA-Py. Is that aligned with what you are thinking?
  • In OATH, there are two scenarios:
    • The automated GHA ACA-Py runs
    • A developer wanting to use a particular version of ACA-Py (perhaps from their own branch), ideally with debug access.
  • The two scenarios would be handled in OATH as we do today, with a separate “dockerfile.acapy-xx” for each scenario.
  • What I was wondering if we could have the OATH “./manage build -a acapy-XX” have:
    • Both scenarios use the “nightly” artifact as the docker FROM image (vs. using a Ubuntu Python image)
    • Both Dockerfiles pull in the ACA-Py backchannel code from GitHub (default — using the “main” branch).
    • The “developer run” Dockerfile would also bring in ACA-Py itself from GitHub (default to the “main” branch, but editable as we can today to use other repos and branches). Loading that would “override” the ACA-Py code already present from the nightly image.

I think that would give us the nightly runs — that would be faster because we wouldn’t be repeating the build process — plus the control needed when a developer needs to run specific tests in OATH against specific versions of the code.

Regards your questions:

Question - There are 2 other supported builds - acapy-redis and dev-acacpy-main, which I'm not sure we actually need or use?

My $0.02CDN — they can be ditched in favour of the “customizable” dockerfile that can be used by a developer. Documentation could help to define scenarios that could set up.

Question - in the OATH ./manage build script, if you leave out the -a argument to build a specific agent, it will build all agents. I think this is incorrect and if -a isn't specified it should build the test harness only and no backchannels?

My $0.02CDN — A long standing bug from when I first created the script. Yes — the lack of a “-a” option should print the usage message and terminate.

Question - is it better to publish a new main backchannel each time the main branch is updated, or is it sufficient to publish on a schedule, e.g. nightly?

My $0.02CDN — We already publish a nightly image and we shouldn’t be publishing other ones. As noted — I would encourage we use the nightly one for OATH automated runs.

Question - do we need to maintain a history of aca-py backchannels, so that new versions can be regression-tested against previous versions?

My $0.02CDN — It’s a pretty obscure edge case. I think if we have the ability to edit the dockerfiles for a specific run, we can do a regression run of the current nightly (one Dockerfile) with an older version by branch/tag/commit (the other Dockerfile). That should be sufficient.

@ianco
Copy link
Contributor

ianco commented Nov 29, 2024

@swcurran we should chat about this next week. In the original comment Follow the pattern of the Aries-VCX backchannel - VCX builds and publishes the backchannel image from the VCX repo. (The OATH tests are still run from the OATH repo.) I expected we'd do the same with aca-py - move the backchannel code, and build (and publish) the backchannel image from the aca-py repo.

If we want to continue to do all the backchannel builds from the OATH repo I'm not sure the value of moving the backchannel code to the aca-py repo.

@nodlesh
Copy link
Contributor Author

nodlesh commented Nov 30, 2024

(Question - There are 2 other supported builds - acapy-redis and dev-acacpy-main, which I'm not sure we actually need or use?)

dev-acapy-main is the base container for the VSCode Dev Container for the ACAPy Backchannel. I use it all the time. Not sure where the dev container fits if the backchannel moves to the acapy repo. Maybe the dev container is useful to move there as well?
Will I, or any other test harness developer, loose access, or make it tedious to update the acapy backchannel if it moves to the acapy repo? During test development and backchannel development I'm switching between the OATH test dev container and the ACAPy Backchannel dev container constantly.

@ianco
Copy link
Contributor

ianco commented Nov 30, 2024

Will I, or any other test harness developer, loose access, or make it tedious to update the acapy backchannel if it moves to the acapy repo? During test development and backchannel development I'm switching between the OATH test dev container and the ACAPy Backchannel dev container constantly.

TBD, depending on how the move is done. I think it will be easier for aca-py developers to test/update the backchannel, but more difficult for test harness developers.

We should chat to determine exactly what we need to do and why. It sounds like the approach taken by VCX is not what we want for aca-py.

@ianco
Copy link
Contributor

ianco commented Dec 2, 2024

After some discussion, we'll leave the backchannel code in the OATH repo (for now anyways) and update the aca-py backchannel builds as follows:

  • add a new acapy-nightly build to build based on the nightly published aca-py image (to speed up build time)
  • change the scheduled test runs to use acapy-nightly instead of acapy-main (to speed up the nightly test runs)
  • update the acapy build to build based on the latest release docker image (instead of building from the pypi image) (to speed up build time)
  • add a new acapy-local build, to build based on local source code (rather than a github project) (so code can be tested locally before committing to github)

Overall this shouldn't impact any work that has to be done in the OATH repo, but will speed up our scheduled tests and make it a bit easier for aca-py developers to manually test with OATH.

@ianco
Copy link
Contributor

ianco commented Dec 2, 2024

FYI it looks like the acapy backchannel image build only takes about a minute, so making the above changes isn't going to shave more than a minute off our build times.

Unless I'm missing something? I purged my local cache and did a few local builds ...

@swcurran
Copy link
Contributor

swcurran commented Dec 2, 2024

Sounds good. Let’s leave it.

@swcurran
Copy link
Contributor

swcurran commented Dec 2, 2024

I was realizing as I looked at a test run this morning — the build takes 99% of the log — probably not 90% of the time. So when I go to look at the results, I have to scroll down to find anything. But I’m sure you are right — it won’t save that much time. Sorry about that.

No need to change — we won’t get a lot of value out of that.

@ianco
Copy link
Contributor

ianco commented Dec 2, 2024

Just did another test with a more thorough purge of local images and cache, and the build took about 2 minutes (including about 30 sec for the test harness). So agree not worth pursuing.

I'll take a look at adding a new build for acapy-local using local source code tho.

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

No branches or pull requests

3 participants