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

Updated cosmos emulator yml script #1365

Merged
5 commits merged into from
Feb 3, 2021

Conversation

kushagraThapar
Copy link
Member

  • Updated cosmos emulator yml script to remove the existing installation, install latest version and run emulator with latest version

…n, install latest version and run emulator with latest version
@kushagraThapar kushagraThapar requested a review from a team as a code owner February 2, 2021 18:44
Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

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

LGTM

- powershell: |
Write-Host "Launching Cosmos DB Emulator"
Import-Module "$env:temp\Azure Cosmos DB Emulator\PSModules\Microsoft.Azure.CosmosDB.Emulator"
Copy link
Member

Choose a reason for hiding this comment

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

@kushagraThapar does the cosmos module not work any longer?

Copy link
Member Author

Choose a reason for hiding this comment

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

@weshaggard - they don't .. I had that conversation with you sometime last month, where we were seeing older versions of cosmos emulator in the azure VMs. So we came up with this script to always install the latest version, since there are some new features in latest version that our code relies on now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or may be it was with Mitch, can't remember, but yes, this was an issue that we found out where cosmos emulator version on the Azure VMs was old, and we couldn't control which version of emulator Azure VMs bring in. They update it every once in a while, but not instantly. So we needed our own script.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was the PR where we first introduced this script - https://github.com/Azure/azure-sdk-for-java/pull/18056/files#r567032679
And then it got overwritten.

Copy link
Member

Choose a reason for hiding this comment

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

OK I'm not super familiar with the cosmos emulator modules but if we are essentially reimplementing them inline here I highly suggest we pull the changes out into a script file so it is easier to run and debug locally. Having a lot of inline code like this makes diagnosing very difficult.

As for the change that got overwritten. We have a particular process of any changes that go under eng/common, that Dozie linked to https://github.com/Azure/azure-sdk-tools/blob/master/doc/common/common_engsys.md. Any changes in there are subject to being overwritten because it is a one-way sync from the tools repo to ensure that all the common tools/scripts are consistent across the different languages.

For these changes I assume this emulator change will work for all the languages and not just java correct? If not we need to also need make the changes to make those work consistently. As part of getting this PR merged into the tools repo we will need to run the cosmos tests in each of the 4 primary language repos in the sync PRs for this change to ensure that these work for those.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the explanation @weshaggard, yes I believe it should work for all languages. @FabianMeiswinkel @milismsft any reason why this change might not work for other languages in case my understanding is not correct ?

Choose a reason for hiding this comment

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

Different languages might use different arguments which control how the Cosmos DB Emulator is started, such as different physical partition count for instance. These arguments have direct impact as to what features are enabled and can be tested, the startup performance of the emulator etc; one setting that fits all it is not practical in this case.

@weshaggard I really have hard time understanding this process of having two separate repos controlling same set of files with no good practices when it comes to sync'ing/merging changes from to another. May be we should completely remove the whole eng/common/pipeline out of this repo, stick a "readme" file instead as where any pipeline related changes should go into what repo. And we would also be better if we have some way to configure those based on the language specifics.

BTW even as of now, there's no solution to practically reproduce a CI issue locally because the images used by the CI are not available to an ordinary user like myself. I will recommend tackling and fixing that first, before we think of the debugging and scripting of these PowerShell scripts.

Copy link
Member

Choose a reason for hiding this comment

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

@milismsft the files under "eng/common" are not supposed to be edited directly in the language repos and we do have a README in that directory calling that out https://github.com/Azure/azure-sdk-for-java/blob/master/eng/common/README.md. We always mirror those files from our tools repo to all the different language repos because they are intended to be shared between all the language repos.

If there are language specific settings or changes they shouldn't be under the eng/common directory, but instead should be somewhere else in the language repository. For this case if the scripts are mostly common we should share as much as we can across the different language teams and provide options to tweak and language specific settings in the language repo's when they consume this template.

Even if we cannot easily run the emulator exactly like we do in CI it is still a good idea to be able to have a script so that we can debug the script itself without running a full CI build to test and find simile code issues in the script.

@check-enforcer-staging
Copy link

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

@benbp
Copy link
Member

benbp commented Feb 2, 2021

@weshaggard do you have an idea why check-enforcer seems to be stalling here?

@benbp
Copy link
Member

benbp commented Feb 2, 2021

/check-enforcer evaluate

@weshaggard
Copy link
Member

/azp where

@azure-pipelines
Copy link

Azure DevOps orgs getting events for this repository:

@weshaggard
Copy link
Member

There appears to be some github/devops status check issues going on. It looks like the PR job was triggered https://dev.azure.com/azure-sdk/internal/_build/results?buildId=715315&view=results but it failed. We need to make sure we get that job passing and that we follow the workflow for testing and syncing these eng/common changes.

@kushagraThapar
Copy link
Member Author

There appears to be some github/devops status check issues going on. It looks like the PR job was triggered https://dev.azure.com/azure-sdk/internal/_build/results?buildId=715315&view=results but it failed. We need to make sure we get that job passing and that we follow the workflow for testing and syncing these eng/common changes.

I see that job is failing because of some timeout / clone issues - https://dev.azure.com/azure-sdk/internal/_build/results?buildId=715315&view=logs&j=96791242-dbf3-587e-3a06-ae5af5c1a705&t=8506b04f-b021-528c-7be1-2b21c2d1a67b&l=159

@weshaggard
Copy link
Member

@chidozieononiwu can you please take a look at https://dev.azure.com/azure-sdk/internal/_build/results?buildId=715315&view=logs&j=96791242-dbf3-587e-3a06-ae5af5c1a705&t=bb13a226-9801-467d-b787-35f80b72d45b to see if you can figure out what is happening there.

@kushagraThapar as for the pipeline checks not updating here I'm on an ICM call with DevOps right now about that issue, it may or may not be related to the issue we are seeing in the build pipeline.

@kushagraThapar
Copy link
Member Author

@chidozieononiwu can you please take a look at https://dev.azure.com/azure-sdk/internal/_build/results?buildId=715315&view=logs&j=96791242-dbf3-587e-3a06-ae5af5c1a705&t=bb13a226-9801-467d-b787-35f80b72d45b to see if you can figure out what is happening there.

@kushagraThapar as for the pipeline checks not updating here I'm on an ICM call with DevOps right now about that issue, it may or may not be related to the issue we are seeing in the build pipeline.

Thanks @weshaggard , appreciate your effort on this. As mentioned earlier, this is a blocking issue for us right now, since we can't get anything checked in. So let me know if you need more justification for the ICM just in case.

@weshaggard
Copy link
Member

@kushagraThapar I'm actively on the ICM bridge I don't think we need more justification. There is a teams thread in our EngSys Team channel we are keeping updated based on this information.

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@weshaggard
Copy link
Member

@kushagraThapar the devops issue has been resolved now you need to follow the eng-common workflow to finish up this PR. I suggest we verify that the cosmos pipelines work in each of the 4 languages and we address any feedback in the inline PS code before going to the next stage of the workflow which creates the sync PRs. If you need help please ping me and I will assist.

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

This looks like a good starting point. As we talked offline it would be good if we can get the inbox emulator modules to support running different versions (i.e. by-pass the registry lookup and just run the emulator next to where the module).

@ghost
Copy link

ghost commented Feb 3, 2021

Hello @azure-sdk!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit b2a154b into Azure:master Feb 3, 2021
@kushagraThapar
Copy link
Member Author

This looks like a good starting point. As we talked offline it would be good if we can get the inbox emulator modules to support running different versions (i.e. by-pass the registry lookup and just run the emulator next to where the module).

Makes sense, not sure if it is possible though, I will let @milismsft comment on that :)

This pull request was closed.
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.

6 participants