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 appstart to spec #13

Merged
merged 2 commits into from
Feb 24, 2023
Merged

add appstart to spec #13

merged 2 commits into from
Feb 24, 2023

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Feb 22, 2023

No description provided.

Signed-off-by: vsoch <[email protected]>
@vsoch
Copy link
Member Author

vsoch commented Feb 22, 2023

@dtrudg looks okay to you too? Anything I left out?

The two tools that implement the spec under this org (I have one in Python, one in Go) I can update after this is merged here. And once we merge into the spec please feel free to continue working on your implementations.

Copy link

@dtrudg dtrudg left a comment

Choose a reason for hiding this comment

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

There is a possible issue with the way this is written, and the proposed implementation in Apptainer / assumed implementation in SingularityCE.

The spec states that....

The minimal set of functions for the controlled must support the following higher level commands.

  • start akin to "run," ....

In Singularity / Apptainer, we have singularity run, but we won't have a top level command start, since that's a subcommand of instance.

The spec kinda infers to me, from the comment about higher level commands that the start command needs to be at the same level as run.

Not sure if you read it the same way, or consider it an issue, but thought I'd bring this up.

Otherwise LGTM.

spec/spec-v1.md Outdated Show resolved Hide resolved
@vsoch vsoch merged commit 2d1dd4a into master Feb 24, 2023
@vsoch vsoch deleted the add/appstart branch February 24, 2023 08:41
@dtrudg dtrudg mentioned this pull request Nov 9, 2023
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