Skip to content

Conversation

@benbroadaway
Copy link
Contributor

There were some changes to the Project API with regard to allowing raw payloads which broke this project. This fixes them for testing with 2.25.0 and newer Concord instances, and keeps backwards compatibility for projects testing against older versions.

  • Fix project creation
  • Add environment variables for overriding docker images
  • JDK17 maven settings

return projectApi.createOrUpdateProject(orgName, projectEntry);
var project = projectApi.createOrUpdateProject(orgName, projectEntry);

if (allowPayloads) {
Copy link
Contributor

@ibodrov ibodrov Apr 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why not set RawPayloadModeEnum.EVERYONE instead?

Copy link
Contributor Author

@benbroadaway benbroadaway Apr 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My rationale: when creating a project in <2.25.0, the default raw payload setting if you only include the "enabled" option, but no level, results in a project that allows payloads from the owner (or an admin). So I was trying to keep that effectively the same.

Unfortunately, doing that involves an extra change -> adding a team to the project, which was not a previous behavior of either this library or the Project API itself.

So something's going to change a little bit either way (EVERYONE vs TEAM). I think I just convinced myself EVERYONE is less of a change even though it's more permissive. If someone has a test that's sensitive to this change, they'd probably hit more unexpectedness from an assumed_team + access_level change than just the access level.

@ibodrov ibodrov merged commit f0bace4 into concord-workflow:master Apr 6, 2025
1 check passed
@benbroadaway benbroadaway deleted the project-2.25.0-compat branch April 7, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants