Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented Sep 15, 2022

What do these changes do?

Adds 3 outputs to the python runner.

Prepare for integration version 1.1.0

Related issue number

How to test

Checklist

  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Update version make version-service-*

@sanderegg sanderegg requested a review from pcrespov September 15, 2022 13:29
@sanderegg sanderegg self-assigned this Sep 15, 2022
@sanderegg sanderegg added this to the vaporwave milestone Sep 15, 2022
@sanderegg sanderegg added the enhancement New feature or request label Sep 15, 2022
def setup():
_ensure_output_subfolders()
logger.info("Processing input from %s:", INPUT_FOLDER)
logger.info("%s", list(INPUT_FOLDER.glob("*")))
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be more informative to have simply a os.system. See this example

>> import os
>> os.system("ls -la")
total 20
drwxrwxr-x 3 crespo crespo 4096 Sep 15 18:28 .
drwxrwxr-x 3 crespo crespo 4096 Jun 24  2021 ..
-rw-rw-r-- 1 crespo crespo 1312 Sep 15 18:28 input_script_1.py
drwxrwxr-x 2 crespo crespo 4096 Sep 15 18:22 __pycache__
-rw-rw-r-- 1 crespo crespo   20 Sep 15 18:10 requirements.txt
0

you can build a context manager to capture the stdout or use something like subprocess.run instead of os.system

logger.info("%s", list(INPUT_FOLDER.glob("*")))

# find entrypoint
user_main_py = _ensure_main_entrypoint(INPUT_FOLDER)
Copy link
Member

Choose a reason for hiding this comment

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

MINOR: re-suggesting a name ;-) ... main_input_path

Copy link
Member Author

Choose a reason for hiding this comment

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

tried another

@sanderegg sanderegg requested a review from pcrespov September 15, 2022 20:02
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

🎉 !! record time!

Please double check my comment with the path before merging ...
Great work!
thx

@sanderegg sanderegg merged commit 80dbc9a into ITISFoundation:master Sep 16, 2022
@sanderegg sanderegg deleted the enhancement/add_4_outputs branch September 16, 2022 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants