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.

Sign up for GitHub

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

Jump to bottom

Serialize layer env #876

Merged
merged 4 commits into from
Jan 22, 2021
Merged

Serialize layer env #876

merged 4 commits into from
Jan 22, 2021

Conversation

splhack
Copy link
Contributor

@splhack splhack commented Jan 9, 2021

Fix #547

  • Layer env can be set from JSON, but it doesn't propagate. Serialize it into XML.
  • Fix layer get_env.

@larsbijl
Copy link
Contributor

Hi @splhack,

Thank you for your contribution!

When I submit a layer with an env variable set, it gets the following traceback:

cuebot_1  | com.imageworks.spcue.SpecBuilderException: Failed to parse job spec XML, org.jdom.input.JDOMParseException: Error on line 1: Attribute value "b'HELLO'" of type NMTOKEN must be a name token.

Here is the example I used to test with

#!/usr/bin/env python
from outline import Outline
from outline.cuerun import OutlineLauncher
from outline.modules.shell import Shell

ol = Outline(name='testing', name_unique=True)
layer1 = Shell('layer1', command=['sleep 5'], range='1-10', threads=1, threadable=True)
layer1.set_env('HELLO', 'WORLD')
ol.add_layer(layer1)

launcher = OutlineLauncher(ol)
jobs = launcher.launch(False)
print(jobs)

@splhack
Copy link
Contributor Author

splhack commented Jan 16, 2021

@larsbijl thank you for testing!
I found out Python2 future.types.newstr.newstr issue on XML via only set_env (JSON is fine.)
Added a workaround, and verified with unittests.

@larsbijl
Copy link
Contributor

Thanks @splhack! tested and works great.

@bcipriano bcipriano merged commit 68f7c1c into AcademySoftwareFoundation:master Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larsbijl larsbijl larsbijl approved these changes

@bcipriano bcipriano bcipriano approved these changes

@DiegoTavares DiegoTavares Awaiting requested review from DiegoTavares DiegoTavares is a code owner

@gregdenton gregdenton Awaiting requested review from gregdenton

@jrray jrray Awaiting requested review from jrray

@smith1511 smith1511 Awaiting requested review from smith1511

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Layer environment variables are not added to XML spec
3 participants
@splhack @larsbijl @bcipriano