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

Added notification message and exception for starting a non-configured endpoint #435

Merged
merged 1 commit into from Apr 15, 2021

Conversation

yongyanrao
Copy link
Contributor

@yongyanrao yongyanrao commented Apr 15, 2021

For Issue #432
After the fix, when we try to start a non-configured endpoint test-ep:

$ funcx-endpoint start test-ep

Endpoint test-ep is not configured!
1. Please create a configuration template with:
	funcx-endpoint configure test-ep
2. Update the configuration
3. Start the endpoint

@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #435 (ba12731) into main (2bb8175) will decrease coverage by 2.65%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #435      +/-   ##
==========================================
- Coverage   35.94%   33.29%   -2.66%     
==========================================
  Files          20       20              
  Lines        2245     2607     +362     
==========================================
+ Hits          807      868      +61     
- Misses       1438     1739     +301     
Impacted Files Coverage Δ
...dpoint/funcx_endpoint/endpoint/endpoint_manager.py 56.62% <ø> (+1.15%) ⬆️
...cx_endpoint/funcx_endpoint/endpoint/interchange.py 28.66% <0.00%> (-3.52%) ⬇️
...ncx_endpoint/executors/high_throughput/executor.py 30.02% <0.00%> (-1.89%) ⬇️
..._endpoint/executors/high_throughput/interchange.py 11.48% <0.00%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bb8175...ba12731. Read the comment docs.

'2. Update the configuration\n'
'3. Start the endpoint\n')
print(msg)
raise FileNotFoundError(f'Endpoint {name} is not configured!')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should probably keep the old style. No need to raise an exception here, and this exception seems irrelevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to raise a similar exception as

FileNotFoundError: [Errno 2] No such file or directory: '/home/zhuozhao/.funcx/test_local_1/config.py'

We can keep the old style.

Copy link
Collaborator

@yadudoc yadudoc left a comment

Choose a reason for hiding this comment

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

This looks good. Let's merge this.

@yadudoc yadudoc merged commit b353896 into main Apr 15, 2021
@yadudoc yadudoc deleted the yrao2-issue-432 branch April 15, 2021 21:38
yongyanrao added a commit that referenced this pull request Apr 16, 2021
yongyanrao added a commit that referenced this pull request Apr 16, 2021
yadudoc added a commit that referenced this pull request Apr 22, 2021
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.

starting non-existing endpoint does not tell people to first configure it
3 participants