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

Fix error response status code when dataset or job already exists #184

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

leonasdev
Copy link

The official BigQuery documentation's error messages state that when creating a job, dataset, or table, a response of 409 should be returned if they already exist.

Currently, I have found that only the table portion has the correct response of 409, while the other two always respond with 500. Therefore, I am submitting this PR to fix this issue.

Additionally, the documentation mentions that The error also returns when a job's writeDisposition property is set to WRITE_EMPTY and the destination table accessed by the job already exists. I have not implemented this part, but it may be considered for future improvements.

This is my first PR of this repo, so please let me know if there are any issues with my PR. Thank you.

@thuibr
Copy link

thuibr commented Dec 5, 2023

I'm running into this too and was just about to consider contributing and fixing it. This is my workaround:

    def create_dataset(self, dataset, exists_ok=False):
        # The exists_ok flag doesn't seem to work with the emulator,
        # so work around it.
        try:
            return self._client.create_dataset(dataset, retry=None)
        except google.api_core.exceptions.InternalServerError as e:
            errors = e.errors
            dataset_basename = dataset.split('.')[-1]
            if exists_ok and re.match(rf'dataset {dataset_basename} is already created', errors[0]["message"]):
                return
            raise e

Obviously, it would be better to fix it in the emulator! Let me know if I can help with anything.

@thuibr
Copy link

thuibr commented Dec 13, 2023

@goccy hi just wondering what I can do to help get this PR through?

"fmt"
"sync"
)

var ErrDuplicatedDataset = errors.New("dataset is already created")
Copy link
Owner

Choose a reason for hiding this comment

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

Please use errDuplicate function in error.go

@@ -691,21 +691,21 @@ type datasetsInsertRequest struct {
dataset *bigqueryv2.Dataset
}

func (h *datasetsInsertHandler) Handle(ctx context.Context, r *datasetsInsertRequest) (*bigqueryv2.DatasetListDatasets, error) {
func (h *datasetsInsertHandler) Handle(ctx context.Context, r *datasetsInsertRequest) (*bigqueryv2.DatasetListDatasets, *ServerError) {
Copy link
Owner

Choose a reason for hiding this comment

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

It does not seem necessary to change to *ServerError type

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.

None yet

3 participants