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

CosmosDB: Added EnableAggregationPipeline to Cosmos DB capabilities #1849

Closed
wants to merge 1 commit into from

Conversation

jamesggraf
Copy link

@jamesggraf jamesggraf commented Aug 30, 2018

I've never written anything in Go and I'm not 100% certain on how to test this so please be gentle :)

Fixes #1848

@ghost ghost added the size/M label Aug 30, 2018
@ghost ghost added the size/M label Aug 30, 2018
@ghost ghost added the size/M label Aug 30, 2018
@tombuildsstuff tombuildsstuff changed the title Added EnableAggregationPipeline to Cosmos DB capabilities #1848 CosmosDB: Added EnableAggregationPipeline to Cosmos DB capabilities #1848 Aug 30, 2018
@tombuildsstuff tombuildsstuff changed the title Added EnableAggregationPipeline to Cosmos DB capabilities #1848 CosmosDB: Added EnableAggregationPipeline to Cosmos DB capabilities #1848 Aug 30, 2018
@tombuildsstuff tombuildsstuff changed the title CosmosDB: Added EnableAggregationPipeline to Cosmos DB capabilities #1848 CosmosDB: Added EnableAggregationPipeline to Cosmos DB capabilities Aug 30, 2018
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @jamesggraf

Thanks for this PR :)

I've taken a look through and this mostly LGTM - if we can add this new Capability to the documentation (and the tests pass) we should be able to get this merged :)

Thanks!

azurerm/resource_arm_cosmos_db_account.go Show resolved Hide resolved
@tombuildsstuff tombuildsstuff added this to the 1.14.0 milestone Aug 30, 2018
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @jamesggraf

Thanks for adding the property to the documentation - this now LGTM, I'll kick off the test suite now 👍

Thanks!

@tombuildsstuff
Copy link
Contributor

hey @jamesggraf

So I've run the test suite for this and it appears this fails because the feature isn't available:

------- Stdout: -------
=== RUN   TestAccAzureRMCosmosDBAccount_aggregation
--- FAIL: TestAccAzureRMCosmosDBAccount_aggregation (1092.50s)
	testing.go:513: Step 0 error: After applying this step, the plan was not empty:
		
		DIFF:
		
		UPDATE: azurerm_cosmosdb_account.test
		  capabilities.#:               "0" => "1"
		  capabilities.2476034231.name: "" => "EnableAggregationPipeline"

Out of interest - do you know if this feature is in an invite-only Beta/Preview?

Thanks!

@jamesggraf
Copy link
Author

Not as far as I know? If I enable that feature through the Preview Features pane in the Cosmos UI I see that capability listed as being enabled. Can you confirm that you see the same thing when you set it? Is this flag just not available through their API?

@jamesggraf
Copy link
Author

Can you confirm that that test is using a MongoDB database under Cosmos?

@dmartosp

This comment has been minimized.

@tombuildsstuff tombuildsstuff modified the milestones: 1.14.0, 1.15.0 Sep 6, 2018
@katbyte
Copy link
Collaborator

katbyte commented Sep 7, 2018

Hi @jamesggraf,

I can confirm the test is using mongo, you can see it here

@jamesggraf
Copy link
Author

Thanks katbyte, is there documentation for how these tests are run? I'd like to better understand why Terraform is having trouble enabling this capability even though I've seen it try to unset it after it's enabled manually through Azure's portal.

@katbyte
Copy link
Collaborator

katbyte commented Sep 9, 2018

@jamesggraf,

The tests are usually run against our test subscription. The runner:

  • does an apply and stands up all the required resources
  • reads back in the state (as normal)
  • does a plan and checks to make sure it is empty
  • does any desired checks on the state
  • and then in this case does an import/read and verifies the imported state matches the current state

The step that is failing here is after applying the test config the plan is still not empty.

@tombuildsstuff tombuildsstuff removed this from the 1.15.0 milestone Sep 13, 2018
@tombuildsstuff tombuildsstuff added this to the 1.16.0 milestone Sep 13, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.16.0, 1.17.0 Sep 25, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.17.0, Soon, Future Oct 11, 2018
@katbyte katbyte added the upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR label Oct 16, 2018
@tombuildsstuff tombuildsstuff modified the milestones: Future, Blocked Oct 25, 2018
@katbyte katbyte self-assigned this Oct 25, 2018
@tombuildsstuff
Copy link
Contributor

hey @jamesggraf

Thanks for this PR - unfortunately at this time this PR is blocked on an upstream issue in the Azure API (which is being tracked here - as such I've currently assigned this PR to the "Blocked" milestone.

Rather than leaving this PR stagnant, I hope you don't mind but I'm going to close this issue temporarily whilst we wait for the issue in the API to be fixed; once the issue in the API is fixed we'll re-open this PR and run the tests/get this merged - however at this time we're unable to proceed with this until the bug in the API is fixed, so I hope you don't mind me temporarily closing this.

Thanks!

@ghost
Copy link

ghost commented Mar 6, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement hashibot/ignore service/cosmosdb size/M upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Support for EnableAggregationPipeline Capability in Cosmos DB (Mongo)
5 participants