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

DURACLOUD-1304 #49

Merged
merged 4 commits into from
Aug 25, 2021
Merged

DURACLOUD-1304 #49

merged 4 commits into from
Aug 25, 2021

Conversation

fozboz
Copy link
Contributor

@fozboz fozboz commented Jul 28, 2021

Testing

Default behaviour

  • Do not change configuration
  • Create a space
  • Workman should send a "New space created" notification email with https://<subdomain>.duracloud.org in the body

Override behaviour

  • Add duracloud-site.domain=other.domain to mill.properties file
  • Restart workman
  • Create a space
  • Workman should send a "New space created" notification email with https://<subdomain>.other.domain in the body

Andy Foster added 2 commits July 28, 2021 10:41
The version of javassist that hibernate includes (3.20.0-GA) was causing
workman to fail to start.
Resolves DURACLOUD-1304.
Copy link
Member

@bbranan bbranan left a comment

Choose a reason for hiding this comment

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

This looks functionally correct. The one thing that jumped out is the unfortunate need to repeat the same code snippet 3 times. I'd prefer to see the code to get the host captured in a method once and reused. I don't see another class where this would fit. What you think about adding a class in org.duracloud.mill.config to do this?

Copy link
Member

@bbranan bbranan left a comment

Choose a reason for hiding this comment

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

The code looks good to me. I've asked @dbernstein to give it a run through.

Copy link
Member

@dbernstein dbernstein left a comment

Choose a reason for hiding this comment

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

I've verified that it works on dev.

@dbernstein dbernstein merged commit 5acd758 into duracloud:develop Aug 25, 2021
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