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

Extends 'external connection schema add' with wait. Closes #5532 #5599

Closed

Conversation

waldekmastykarz
Copy link
Member

Extends 'search externalconnection schema add' with wait. Closes #5532

@martinlingstuyl
Copy link
Contributor

Hi @waldekmastykarz, today we merged a PR with renames of the entire external command group. Hence the merge conflicts... could you maybe fix them?

@waldekmastykarz
Copy link
Member Author

On it!

@waldekmastykarz waldekmastykarz marked this pull request as draft November 9, 2023 18:50
@waldekmastykarz waldekmastykarz force-pushed the externalconnection-add-wait branch 2 times, most recently from 93f6487 to c040fa5 Compare November 10, 2023 07:47
@waldekmastykarz waldekmastykarz marked this pull request as ready for review November 10, 2023 07:47
@waldekmastykarz waldekmastykarz changed the title Extends 'search externalconnection schema add' with wait. Closes #5532 Extends 'external connection schema add' with wait. Closes #5532 Nov 10, 2023
@Jwaegebaert Jwaegebaert self-assigned this Nov 23, 2023
@Jwaegebaert Jwaegebaert added the hacktoberfest-accepted Accept for hacktoberfest, will merge later label Nov 23, 2023
Copy link
Contributor

@Jwaegebaert Jwaegebaert left a comment

Choose a reason for hiding this comment

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

I've nothing to add regarding the --wait functionality. Works smoothly @waldekmastykarz.

I'm a bit curious about the second addition you mentioned in the issue. Is that something you want to track in a separate issue and first implement the await logic?

@waldekmastykarz
Copy link
Member Author

Good catch @Jwaegebaert, seems like I totally lost track of it. Sending the location to stdout is a small change, so let's bring it in straight away. Let me update the PR

@waldekmastykarz waldekmastykarz marked this pull request as draft November 27, 2023 09:46
@waldekmastykarz waldekmastykarz force-pushed the externalconnection-add-wait branch from c040fa5 to f6f7cef Compare November 28, 2023 07:00
@waldekmastykarz waldekmastykarz marked this pull request as ready for review November 28, 2023 07:00
Copy link
Contributor

@Jwaegebaert Jwaegebaert left a comment

Choose a reason for hiding this comment

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

Pardon for the delay but awesome work @waldekmastykarz. One note I've is that we should mention in the docs that we'll return the job status URL in case they want to follow it up manually. I can add this manually during the merging, I was thinking something along the lines of this:

## Response

Upon executing the command, you'll receive the job status URL. This URL enables manual checking of the job's progress if automated polling fails or for interactive monitoring purposes.

```text
"https://graph.microsoft.com/v1.0/external/connections/MyApp/operations/795b6888-4093-0fe7-6270-ddbcac9ebd3a"
```

@waldekmastykarz
Copy link
Member Author

All good. Great idea @Jwaegebaert. Please do

@waldekmastykarz waldekmastykarz deleted the externalconnection-add-wait branch December 29, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accept for hacktoberfest, will merge later pr-merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend 'search externalconnection schema add' with wait
3 participants