Skip to content

MATLAB: allow specifying the license file directly#1712

Merged
migueldiascosta merged 2 commits intoeasybuilders:developfrom
smoors:matlab
Jul 22, 2019
Merged

MATLAB: allow specifying the license file directly#1712
migueldiascosta merged 2 commits intoeasybuilders:developfrom
smoors:matlab

Conversation

@smoors
Copy link
Copy Markdown
Contributor

@smoors smoors commented May 6, 2019

No description provided.

migueldiascosta
migueldiascosta reviewed May 7, 2019
View reviewed changes
Comment thread easybuild/easyblocks/m/matlab.py Outdated
@@ -74,15 +74,16 @@ def configure_step(self):
licport = self.cfg['license_server_port']
if licport is None:
licport = os.getenv('EB_MATLAB_LICENSE_SERVER_PORT', '00000')
Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta May 7, 2019

Choose a reason for hiding this comment

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

for consistency, everything regarding licserv and licport should also be inside if licfile is None: then, since it's only used there?

@smoors smoors requested a review from migueldiascosta May 7, 2019 08:37
@Micket
Copy link
Copy Markdown
Contributor

Micket commented May 13, 2019

While, not a huge problem, we are a bit inconsistent when it comes to specifying licenses.
Rather than hardcoding anything into any configs, shouldn't we just keep environment variables for all of them?
Among software I've installed, I found;

export LMCOMSOL_LICENSE_FILE=...
export INTEL_LICENSE_FILE=...
export EB_ABAQUS_LICENSE_FILE=...

I think we should, at least, allow for the use of export EB_MATLAB_LICENSE_FILE=...

@boegel boegel added this to the 3.x milestone May 16, 2019
@damianam
Copy link
Copy Markdown
Member

damianam commented May 28, 2019 •
edited
Loading

@Micket I don't disagree with you, but I think this might be better suited for a broader license related PR. This one does just one thing, and seems to do it well. I see no reason to don't merge it.

I'll wait for your reply to merge it. Would you agree @migueldiascosta ?

@bartoldeman
Copy link
Copy Markdown
Contributor

bartoldeman commented Jul 9, 2019

@migueldiascosta ping?

migueldiascosta
migueldiascosta approved these changes Jul 10, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

lgtm

@migueldiascosta
Copy link
Copy Markdown
Member

migueldiascosta commented Jul 22, 2019

Going in, thanks @smoors!

@migueldiascosta migueldiascosta merged commit 624cffd into easybuilders:develop Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@migueldiascosta migueldiascosta migueldiascosta approved these changes

@damianam damianam damianam approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

3.9.4

Development

Successfully merging this pull request may close these issues.

6 participants

@smoors @Micket @damianam @bartoldeman @migueldiascosta @boegel