Python unit tests are failing on pull requests from forks

[jpower432]

Describe the bug

CI variables are not accessible by forks on the pull_request trigger. Tests are failing on pull requests from forks because repository variables are not accessible.

To Reproduce

Steps to reproduce the behavior:

1. Create a branch in fork
2. Open a PR
3. Observe error

Expected behavior

Tests to pass

Screenshots / Logs.

Related failing PR:

1621

Environment

• OS: [e.g. iOS]
• Python version:
• Installed packages:

[jpower432]

@vikas-agarwal76 @butler54 I believe this is stems from the changes on https://github.com/oscal-compass/compliance-trestle/pull/1612. I recently found that repository variables are not accessible to forks. Using environment variables is also not an option in the matrix field. Looking to brainstorm some solutions. cc @gvauter

[butler54]

Reverting to some hard coded vars is the obvious choice, however, it's kind of janky that you are still defining the variables multiple times.

My preference would be to read is from a config file so what we test is what we tell the world via pypi.

Thoughts?

[jpower432]

Reverting to some hard coded vars is the obvious choice, however, it's kind of janky that you are still defining the variables multiple times.

@butler54 I agree. My goal was to avoid reverting the change back to the hard-coded values.

My preference would be to read is from a config file so what we test is what we tell the world via pypi.

It looks like the matrix can be set from job outputs. Using that with fromJSON gives us a lot more options including a config file approach. I am assuming you mean from read the classifiers or python_require from the setup.cfg and do some basic processing logic to set min and max versions. Is that correct?

For a quicker turnaround, I think the simplest approach could be to set workflow level environment variables and use a job to assemble that into an output that could consumed by other jobs. That would result in duplication between the workflows, but would unblock #1621 and other contributions from forks.

What do you think?

[butler54]

@jpower432

I am assuming you mean from read the classifiers or python_require from the setup.cfg and do some basic processing logic to set min and max versions. Is that correct?

100% but this will require quite a bit more work. It also opens up the question of setup.cfg vs pyproject which is worthwhile discussing OoB to this issue.

For a quicker turnaround, I think the simplest approach could be to set workflow level environment variables and use a job to assemble that into an output that could consumed by other jobs. That would result in duplication between the workflows, but would unblock https://github.com/oscal-compass/compliance-trestle/pull/1621 and other contributions from forks.

As we have multiple workflows my thought is this:

1. We define PYTHON_MIN and PYTHON_MAX in a json config file
2. We use cat and fromJson to load it into the environment consistently in each workflow as the first step in the workflow.

It's slightly janky, however, I think it definitely is as close as where we are now but does allow integration into forks.

it also opens up another angle: python versions in setup.cfg could be derived from / validated by the json config file. This (from a CICD workflow perspective) might be easier than reading the setup.cfg file. This last part can be explored later.

[jpower432]

@butler54

100% but this will require quite a bit more work. It also opens up the question of setup.cfg vs pyproject which is worthwhile discussing OoB to this issue.

Agree. I was thinking that one of the more time consuming parts of the solution was parsing setup.cfg. Happy to discuss pyproject.toml in another issue or GH Dicussion.

As we have multiple workflows my thought is this:

1. We define PYTHON_MIN and PYTHON_MAX in a json config file
2. We use cat and fromJson to load it into the environment consistently in each workflow as the first step in the workflow.

It's slightly janky, however, I think it definitely is as close as where we are now but does allow integration into forks.

Yes. This sounds good and should be just as simple to implement as env variables with reduced duplication. I am happy to work on this and submit a PR for review.

it also opens up another angle: python versions in setup.cfg could be derived from / validated by the json config file. This (from a CICD workflow perspective) might be easier than reading the setup.cfg file. This last part can be explored later.

[butler54]

I'll try to do something EoW if that is okay.

[jpower432]

@butler54 Sounds good to me. I played around with this a bit on my fork before you responded. Feel free to leverage anything there - https://github.com/jpower432/compliance-trestle/tree/ci/fork-integration