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

call metadata api after uploading file #2114

Merged
merged 1 commit into from
Dec 11, 2024
Merged

call metadata api after uploading file #2114

merged 1 commit into from
Dec 11, 2024

Conversation

QingengWei
Copy link

No description provided.

@momchil-flex
Copy link
Collaborator

Thanks @magiWei

It seems that some tests are failing because they are actually trying to connect to the server to estimate the cost? We need to mock that function in the tests.

The other thing is, we also discussed adding a check estimate api call in web.start before the submit api is called. Or maybe it should even be added directly in task.submit. And then this function also needs to be mocked in the tests.

@QingengWei
Copy link
Author

Thanks @magiWei

It seems that some tests are failing because they are actually trying to connect to the server to estimate the cost? We need to mock that function in the tests.

The other thing is, we also discussed adding a check estimate api call in web.start before the submit api is called. Or maybe it should even be added directly in task.submit. And then this function also needs to be mocked in the tests.

About #2, it will check on API server side.

@momchil-flex
Copy link
Collaborator

Thanks @magiWei
It seems that some tests are failing because they are actually trying to connect to the server to estimate the cost? We need to mock that function in the tests.
The other thing is, we also discussed adding a check estimate api call in web.start before the submit api is called. Or maybe it should even be added directly in task.submit. And then this function also needs to be mocked in the tests.

About #2, it will check on API server side.

Ah ok makes sense.

For the tests I think you also need to add mock_get_info.

@QingengWei
Copy link
Author

Thanks @magiWei
It seems that some tests are failing because they are actually trying to connect to the server to estimate the cost? We need to mock that function in the tests.
The other thing is, we also discussed adding a check estimate api call in web.start before the submit api is called. Or maybe it should even be added directly in task.submit. And then this function also needs to be mocked in the tests.

About #2, it will check on API server side.

Ah ok makes sense.

For the tests I think you also need to add mock_get_info.

What is the most recent release version? Can we merge this feature into it?

@momchil-flex
Copy link
Collaborator

We are preparing a 2.8 pre-release right now. Normally we're not planning on making more 2.7 patches.

@momchil-flex
Copy link
Collaborator

@magiWei I also added a commit to remove the estimated cost message from web.monitor as now it will be duplicated during web.run. What do you think?

We should also add a changelog item, I can do it when we get to a final state of this.

@QingengWei
Copy link
Author

QingengWei commented Dec 10, 2024

@magiWei I also added a commit to remove the estimated cost message from web.monitor as now it will be duplicated during web.run. What do you think?

We should also add a changelog item, I can do it when we get to a final state of this.

It makes sense. I have no new commit.

@momchil-flex momchil-flex force-pushed the SCEM-8423 branch 3 times, most recently from 3f0115f to a7d0454 Compare December 11, 2024 13:43
@momchil-flex momchil-flex merged commit 95b8fc1 into pre/2.8 Dec 11, 2024
14 checks passed
@momchil-flex momchil-flex deleted the SCEM-8423 branch December 11, 2024 14:09
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.

2 participants