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

Add outputs #1361

Merged
merged 6 commits into from
Jun 11, 2024
Merged

Conversation

lithorus
Copy link
Contributor

Link the Issue(s) this Pull Request is related to.
#732

Summarize your change.

  • Added output element to new xml spec
  • Used output elements to populate database
  • Fetch outputs from database and add to job spec

@lithorus
Copy link
Contributor Author

Btw. one thing I noticed was that when adding outputs in the API, you specify name and path of the output, however on the server side (registerLayerOutput()) it doesn't really have the functionaility of keeping the name, only the path.
Should I look at implementing this?

@DiegoTavares
Copy link
Collaborator

Btw. one thing I noticed was that when adding outputs in the API, you specify name and path of the output, however on the server side (registerLayerOutput()) it doesn't really have the functionaility of keeping the name, only the path. Should I look at implementing this?

Interesting, I haven't noticed that. It can either be integrated under this PR or you can create a new one if you want.

@lithorus
Copy link
Contributor Author

Probably best in another PR. Atleast it shouldn't need updates to the spec and the current implementation solves the need for the issue linked in here.

@DiegoTavares DiegoTavares merged commit c29e445 into AcademySoftwareFoundation:master Jun 11, 2024
12 checks passed
@lithorus lithorus deleted the add-outputs branch June 15, 2024 20:13
DiegoTavares pushed a commit that referenced this pull request Jun 17, 2024
When importing the changes for "ass-outputs" I was missing the function to parse the output data from the jobspec. Follow up to #1361
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.

None yet

2 participants