Added checks for dataset registration #200
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #200 +/- ##
==========================================
+ Coverage 21.26% 21.41% +0.15%
==========================================
Files 35 35
Lines 1632 1639 +7
==========================================
+ Hits 347 351 +4
- Misses 1275 1277 +2
- Partials 10 11 +1
Continue to review full report at Codecov.
|
myungjin
requested changes
Jul 29, 2022
dhruvsgarg
force-pushed
the
updated_dataset_reg
branch
from
725a08b
to
2410f20
Compare
dhruvsgarg
changed the title
myungjin
reviewed
Aug 1, 2022
dhruvsgarg
force-pushed
the
updated_dataset_reg
branch
from
2410f20
to
743b001
Compare
myungjin
reviewed
Aug 3, 2022
This pr maps the realm in the input dataset json to the registered computes. The realm in the dataset json specifies either region OR computeId. If the dataset is private, the specified region or computeId must be correct. If it is invalid, an error would be thrown and creation of the dataset would fail. In the case of public datasets, if the region or computeId does not match, the defaultRealm is selected which also specifies a default computeId. The dataset realm is used at the time of task creation to assign computeIds and the job tasks get assigned to that compute cluster.
dhruvsgarg
force-pushed
the
updated_dataset_reg
branch
from
743b001
to
ab823e5
Compare
myungjin
approved these changes
Aug 4, 2022
| @@ -110,3 +141,41 @@ func (s *DatasetsApiService) UpdateDataset(ctx context.Context, user string, dat | |||
|
|
|||
| return openapi.Response(http.StatusNotImplemented, nil), errors.New("UpdateDataset method not implemented") | |||
| } | |||
|
|
|||
| func (s *DatasetsApiService) checkComputeForDatasetByRegion(datasetRealm string, datasetInfo *openapi.DatasetInfo) error { | |||
| eligibleComputes, err := s.dbService.GetComputeIdsByRegion(datasetRealm) | |||
There was a problem hiding this comment.
There is no check on err. Is this safe or can the error be ignored? If so, you may need to change it to something like the following:
eligibleComputes, _ := s.dbService.GetComputeIdsByRegion(datasetRealm)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pr maps the realm in the input dataset json to the registered computes. The realm in the dataset json specifies either region OR computeId. If the dataset is private, the specified region or computeId must be correct. If it is invalid, an error would be thrown and creation of the dataset would fail. In the case of public datasets, if the region or computeId does not match, the defaultRealm is selected which also specifies a default computeId. The dataset realm is used at the time of task creation to assign computeIds and the job tasks get assigned to that compute cluster.