Skip to content

Store config in local bucketing from config manager - #22

Merged
emmawillis merged 1 commit into
java-local-bucketing-changesfrom
DVC-4206-config2
Sep 8, 2022
Merged

Store config in local bucketing from config manager#22
emmawillis merged 1 commit into
java-local-bucketing-changesfrom
DVC-4206-config2

Conversation

@emmawillis

Copy link
Copy Markdown
Contributor

No description provided.

@emmawillis
emmawillis requested review from a team and JamieSinn September 8, 2022 14:41
localBucketing.storeConfig(environmentKey, mapper.writeValueAsString(config));
} catch (JsonProcessingException e) {
errorResponse.setMessage(e.getMessage());
throw new DVCException(HttpResponseCode.byCode(500), errorResponse);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this shouldn't be a 500 response code since its a parsing error and not an issue with the http request

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh true, the DVCException requires a HttpResponseCode though - i guess i could drop that and throw the JsonProcessingException directly?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yup, we should also fall back to using the previous config (if available) and not erroring out the process

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed!

@emmawillis
emmawillis merged commit ca1b42c into java-local-bucketing-changes Sep 8, 2022
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