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

Document machine-to-machine auth options in openeo #613

Closed
wants to merge 4 commits into from

Conversation

soxofaan
Copy link
Collaborator

ref: eu-cdse/openeo-cdse-infra#233

- Likewise, the balances of processing credits are separate.
It is however possible to link the balance of the client credentials account
to your personal account.
To enable that: contact support and provide your client id and user id.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JanssenBrm @rahmandawibowo-vito this probably needs some more work. I'm not sure what you need nowadays at the marketplace side of things to link client credential accounts with user accounts

Copy link

github-actions bot commented Sep 11, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://eu-cdse.github.io/documentation/pr-preview/pr-613/
on branch gh-pages at 2024-09-16 13:47 UTC

Copy link
Collaborator

@Pratichhya Pratichhya left a comment

Choose a reason for hiding this comment

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

I assume as a first version of the document, it is good to be merged.

Minor changed in the sentences with regards to pronoun "you" has been done

APIs/openEO/authentication.qmd Outdated Show resolved Hide resolved
and discusses alternative authentication methods.


## Non-interactive And Machine-to-Machine Authentication with the openEO Python client library
Copy link
Collaborator

Choose a reason for hiding this comment

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

This too seems a bit longer title to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To make it easy for people to find this I wanted to include "non-interactive" and "machine-to-machine" in the title. It also focuses heavily on python use cases, so I wanted to make that clear

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

made it bit shorter

@Pratichhya
Copy link
Collaborator

@KanerLev @Slegersj @bomansb FYI

@soxofaan
Copy link
Collaborator Author

Minor changed in the sentences with regards to pronoun "you" has been done

@Pratichhya just curious: what is the background of removing "you"? is that a style guide?

I also noticed you removed quite some newlines from my text, is that intentional?
Note that I intentionally use newlines in docs, following https://sembr.org/

@Pratichhya
Copy link
Collaborator

Pratichhya commented Sep 16, 2024

Minor changed in the sentences with regards to pronoun "you" has been done

@Pratichhya just curious: what is the background of removing "you"? is that a style guide?

@soxofaan, the reason to update sentences having "you" or "your" as pronouns was to maintain the idea adopted in PR #533 as suggested by @HansVRP. To limit the use of "you", instead, use "user" for consistent and academic writing.

I also noticed you removed quite some newlines from my text, is that intentional? Note that I intentionally use newlines in docs, following https://sembr.org/

Ahh, I am extremely sorry about the newlines style. I didn't know they were intentional and thought the markdown anyway read them as a single line. I will try to update these 😅

@soxofaan
Copy link
Collaborator Author

soxofaan commented Sep 16, 2024

the reason to update sentences having "you" or "your" as pronouns was to maintain the idea adopted in PR #533 as suggested by @HansVRP. To limit the use of "you", instead, use "user" for consistent and academic writing.

Ok good to know, makes sense to add this to the guidelines (#612) I guess

@soxofaan
Copy link
Collaborator Author

thought the markdown anyway read them as a single line

Yes that's the point: from the standpoint of Markdown tools they are considered a single paragraph.
But the separate lines are much friendlier to other tools like git, GitHub pull requests, ...

@Pratichhya
Copy link
Collaborator

thought the markdown anyway read them as a single line

Yes that's the point: from the standpoint of Markdown tools they are considered a single paragraph. But the separate lines are much friendlier to other tools like git, GitHub pull requests, ...

Noted, thank you for the suggestion. I updated the changed sentences in the PR and will try to implement this in my other PRs too :)

@soxofaan
Copy link
Collaborator Author

Are there any open issues here I have to address or other todo's to get this merged?

@KanerLev
Copy link
Collaborator

KanerLev commented Sep 23, 2024

Hi @soxofaan, I couldn't write earlier. Content-wise all OK from our side however to get it merged now please commit them directly to publish (not staging).

With the current operations flow we follow:
-all PRs are committing changes directly to publish e.g. see https://github.com/eu-cdse/documentation/pulls.
-staging is rebased afterwards

We are working with all stakeholders to introduce committing to staging before carrying them to public :
i.e.

  • we commit to staging during a timeframe e.g. for a week
  • e.g. once in every week we commit what's accumulated in staging to public

however we encounter several bottlenecks and still need time on them:

  1. on the timeframe (some changes are asked to be public within hours/days)
  2. on the technical level: staging and the staff need to be prepared for such flow.

If that doesn't suit you, let me know if it suits you that I create a new PR and commit these directly to publish.

Any other feedback / suggestions please share

Copy link
Collaborator

@KanerLev KanerLev left a comment

Choose a reason for hiding this comment

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

Comment provided

@soxofaan
Copy link
Collaborator Author

Ok I'll rebase against the publish branch

However note that I expliclity chose staging because it is documented that way in the README:

documentation/README.md

Lines 16 to 17 in fe8c3b0

## Other changes
All other changes are made via a PR on the `staging` branch.

What is also not clear:

please commit them directly to publish (not staging).

do you really mean I should commit to the publish branch? Or do you mean that I should work in feature branch, branching off of publish and create a pull request targetting publish?

@KanerLev
Copy link
Collaborator

KanerLev commented Sep 23, 2024

Ok I'll rebase against the publish branch

However note that I expliclity chose staging because it is documented that way in the README:

documentation/README.md

Lines 16 to 17 in fe8c3b0

## Other changes
All other changes are made via a PR on the `staging` branch.

What is also not clear:

please commit them directly to publish (not staging).

do you really mean I should commit to the publish branch? Or do you mean that I should work in feature branch, branching off of publish and create a pull request targetting publish?

On README: Indeed, thank you for highlighting, I'll edit

On "commit them directly to publish (not staging)": Thanks for asking constructively, I meant the second option ("that I should work in feature branch, branching off of publish and create a pull request targetting publish").

soxofaan added a commit that referenced this pull request Sep 24, 2024
@soxofaan
Copy link
Collaborator Author

I rebased this pull request targeting the publish branch in PR #624

@soxofaan soxofaan closed this Sep 24, 2024
@soxofaan soxofaan mentioned this pull request Sep 30, 2024
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.

3 participants