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

FISH-477 Deployment group properties are now resolved #4850

Merged

Conversation

Cousjava
Copy link
Contributor

Description

This is a bug fix

Important Info

Blockers

Testing

Testing Performed

Manual tests, see Jira for reproducer

Testing Environment

Zulu JDK 1.8_212 on Ubuntu 20.04 with Maven 3.6.3

@Cousjava Cousjava added this to the 5.2020.5 milestone Aug 24, 2020
@Cousjava
Copy link
Contributor Author

Jenkins test please

@Cousjava Cousjava requested review from pdudits and fturizo August 28, 2020 11:53
// so we need to add System Properties in *reverse order* to get the
// right precedence.

List<SystemProperty> domainSPList = domain.getSystemProperty();
List<SystemProperty> configSPList = getConfigSystemProperties();
Cluster cluster = server.getCluster();
List<SystemProperty> clusterSPList = null;

List<DeploymentGroup> depGroups = server.getDeploymentGroup();
Copy link
Member

Choose a reason for hiding this comment

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

Can this be null?
Having trouble reproducing atm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This cannot be null, but the list may be of length 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This cannot be null, but is can be a 0-sized list.

@jGauravGupta
Copy link
Contributor

Jenkins test please

Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

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

This gets validated twice, once against the DAS config, and once against the target config - meaning you need to have the property set on both configs for it to pick it up. Ideally this should validate it against just the target config.

@Cousjava Cousjava changed the title FISH-275 Deployment group properties are now resolved FISH-477 Deployment group properties are now resolved Sep 28, 2020
@Cousjava Cousjava force-pushed the FISh-275-deploymentgroup-properties branch from 4d25834 to 745ed3a Compare September 28, 2020 09:04
@Cousjava Cousjava requested a review from Pandrex247 September 28, 2020 09:04
@Cousjava
Copy link
Contributor Author

Jenkins test please

@Cousjava Cousjava merged commit aa29d16 into payara:master Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants