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

feat: make app_id a public API #137

Conversation

maartenbreddels
Copy link

Fixes #127

@maartenbreddels maartenbreddels requested a review from a team July 14, 2022 09:42
@maartenbreddels
Copy link
Author

Fixes #128

I decided to push two commits to this PR, since the seconds depends on the first.

@ddl-olsonJD
Copy link
Contributor

ddl-olsonJD commented Jul 26, 2022

Hello @maartenbreddels I will be looking into this.
Do you also have some test cases for this?

@ddl-olsonJD ddl-olsonJD self-requested a review July 26, 2022 17:36
@maartenbreddels
Copy link
Author

Hi,

Happy to add them!
let me know what kind of test I could use as a template.

Regards,

Maarten

Copy link
Contributor

@ddl-olsonJD ddl-olsonJD left a comment

Choose a reason for hiding this comment

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

And for tests, you can follow the test suites that we already have, but around app publishing and un-publishing and examples of how you would use this change.

self.log.debug(f"App {app_id} status={status}")
if status and status != "Stopped" and status != "Failed":
url = self._routes.app_stop(app_id)
response = self.request_manager.post(url)
return response

def __app_get_status(self, id) -> Optional[str]:
app_id = self._app_id
def app_id(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Set as a property as well.

# for backwards compatibility, we keep this property
return self.app_id()

def app_get_status(self, id) -> Optional[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

def def app_get_status(self, ext_app_id=None) -> Optional[str]:
    app_id = ext_app_id if ext_app_id else self.app_id

if app_id is None:
return None
url = self._routes.app_get(app_id)
response = self.request_manager.get(url).json()
return response.get("status", None)

def __app_get_status(self, id) -> Optional[str]:
# for backwards compatibility, we keep this method
return self.__app_get_status()
Copy link
Contributor

Choose a reason for hiding this comment

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

meant to call app_get_status?

@ddl-olsonJD
Copy link
Contributor

ddl-olsonJD commented Jan 3, 2023

@ddl-olsonJD
Copy link
Contributor

close as out of date

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.

Can we make Domino._app_id a public API?
2 participants