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

Combinig cosmos db samples #708

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

Conversation

M-Lipin
Copy link

@M-Lipin M-Lipin commented Jul 5, 2023

Purpose

This PR solves the issue #577.

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

The change is a refactoring: combining two Azure spring boot samples (for AKS and a regular one) into one sample.

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Need to review readme files and try Azure spring boot sample with AKS and without it.
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install

What to Check

Verify that the following are valid
Readme files for Azure spring boot samples.

Other Information

There were some differences between AKS and non-AKS Azure spring boot samples. Need to verify that the change does not break anything.

@M-Lipin M-Lipin requested a review from a team as a code owner July 5, 2023 16:42
public class CosmosSampleApplication implements CommandLineRunner {

private static final Logger LOGGER = LoggerFactory.getLogger(CosmosSampleApplication.class);
public class CosmosdbSpringApplication implements CommandLineRunner {

Choose a reason for hiding this comment

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

Suggested change
public class CosmosdbSpringApplication implements CommandLineRunner {
public class CosmosDBSpringApplication implements CommandLineRunner {

Copy link
Author

Choose a reason for hiding this comment

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

Fixed. Thanks!

Assert.isTrue(optionalUserResult.isPresent(), "Cannot find user.");

final User result = optionalUserResult.get();
Assert.state(result.getFirstName().equals(testUser.getFirstName()), "query result firstName doesn't match!");
Assert.state(result.getLastName().equals(testUser.getLastName()), "query result lastName doesn't match!");

LOGGER.info("findOne in User collection get result: {}", result.toString());
LOGGER.info("spring-cloud-azure-data-cosmos-sample successfully run.");
LOGGER.info("spring-cloud-azure-data-cosmos-sample-aks successfully run.");

Choose a reason for hiding this comment

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

why are we adding -aks here?

Copy link
Author

@M-Lipin M-Lipin Jul 7, 2023

Choose a reason for hiding this comment

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

Was added accidentally when moving from aks sample. Removed. Thanks!

@Netyyyy
Copy link
Contributor

Netyyyy commented Jul 13, 2023

@M-Lipin Please take a look at the case where the check fails, thanks.

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