Skip to content

Add method for validate-templates endpoint#271

Merged
dekatzenel merged 3 commits intomasterfrom
feature/PLA-3369
Apr 1, 2020
Merged

Add method for validate-templates endpoint#271
dekatzenel merged 3 commits intomasterfrom
feature/PLA-3369

Conversation

@dekatzenel
Copy link
Copy Markdown
Contributor

Citrine Python PR

Description

Add method to hit backend endpoint /validate-templates

PR Type:

  • Breaking change (fix or feature that would cause existing functionality to change)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Adherence to team decisions

  • I have added tests for 100% coverage
  • I have written Numpy-style docstrings for every method and class.
  • I have communicated the downstream consequences of the PR to others.

@dekatzenel dekatzenel requested a review from anduill April 1, 2020 00:41
Copy link
Copy Markdown

@anduill anduill left a comment

Choose a reason for hiding this comment

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

this seems like a simple PR. I don't see anything glaring. This just hits the endpoint for validation right? I don't think I saw anything else.

@dekatzenel
Copy link
Copy Markdown
Contributor Author

this seems like a simple PR. I don't see anything glaring. This just hits the endpoint for validation right? I don't think I saw anything else.

There's also a couple of small tweaks to how testing works more broadly in session.py

Copy link
Copy Markdown

@anduill anduill left a comment

Choose a reason for hiding this comment

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

please mark this as alpha. Afterwards, I, David Garcia, will approve this Pull Request.

@dekatzenel
Copy link
Copy Markdown
Contributor Author

please mark this as alpha. Afterwards, I, David Garcia, will approve this Pull Request.

Upon reflection, this is beta because the core backend business logic is already established and well-tested

@dekatzenel dekatzenel requested a review from anduill April 1, 2020 16:29
@bfolie
Copy link
Copy Markdown

bfolie commented Apr 1, 2020

Upon reflection, this is beta because the core backend business logic is already established and well-tested

I don't think the [Beta] tag is being used in citrine-python (since it's 0.x.x, isn't everything beta?). But I agree it's not alpha, since unlike Ara we don't expect it to be volatile.

Copy link
Copy Markdown

@anduill anduill left a comment

Choose a reason for hiding this comment

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

Ok, I approve this with the understanding that this new client feature can be expected to always work. (i.e. it's marked as BETA). Any backend services that this feature depends on are also BETA or better. If there is ANY doubt on these invariants, it might be better to mark this as ALPHA instead.

@dekatzenel dekatzenel merged commit 236601d into master Apr 1, 2020
@dekatzenel dekatzenel deleted the feature/PLA-3369 branch April 1, 2020 16:57
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