supported-series metadata key name provides a poor user experience #156

Open
marcoceppi opened this Issue Sep 28, 2015 · 12 comments

Comments

Projects
None yet
5 participants
Owner

marcoceppi commented Sep 28, 2015

I decided to open an issue instead of commenting on an already closed pull req #155. Not sure why this was changed given series already existed, was not being used, and conveys accurately what the field is designed for given the scope of the file. This feels tantamount to renaming the name field to charm-name. Before v6 moves into stable I'd like to discuss and hopefully reverse this decision.

Owner

rogpeppe commented Sep 28, 2015

The problem is that the series field was accepted in the previous charm version,
and although no-one used it, it is still serialised when the metadata is turned
into JSON (for example when passing it over the juju API), so we cannot use
the Series name when marshaling JSON because it would constitute a backwardly
incompatible change.

An alternative to PR #155 would be to change the serialisation field name for JSON
only (for example to SupportedSeries) so that there would be no conflict
in that case.

Owner

marcoceppi commented Sep 28, 2015

Thanks for the explanation @rogpeppe, that makes sense. I'd opt for the alternative case you outlined, which I assume would mean series key would map to SupportedSeries in the golang code which would leave the previous Series intact. I understand the need now to rename, but given that series was never surfaced to a user we wouldn't need to worry about the backwards compat of that user facing key.

Member

johnsca commented Sep 28, 2015

Are we changing the meaning of the previously accepted field? It seems like we're still talking about the same bit of metadata, we're just extending it to allow for multiple series to be supported. Couldn't we just define it as accepting either a single value or a list?

Member

johnsca commented Sep 28, 2015

Also, what compatibility are we trying to maintain for an unused field? That seems contradictory to me.

Owner

rogpeppe commented Sep 28, 2015

@marcoceppi I was actually talking about the JSON serialisation, not the Go field name (which we've decided can change - we're deliberately introducing this as a non-backwardly-compatible package update).

My proposal is that field in the JSON serialisation (which is something that happens on the wire, not usually visible to juju users) gets named as "SupportedSeries" (it was "Series" before), and then there should be no issue, and no need to add awkward code for unmarshaling either as a list or a single value which I'd prefer to avoid if possible.

@johnsca I am not concerned that actual users are using the "series" field in real charm metadata, but it is an issue that previous juju releases will produce JSON data with a "Series":"" field.

Owner

mitechie commented Sep 28, 2015

I just want to flesh out the end goals for the use here that I think we should try to keep to.

With this change, since we're not going to be making this a backward incompatible change for Juju itself (moving to 2.0) Juju needs to be able to locally deploy both the older series (single string) and the new list version.

What would be ideal is if Juju could output a deprecation notice to users if they were to juju deploy an older format charm. Has there been any thought or work put towards this type of communication to users?

As for the charmstore, we can make a policy update that we perform some sort of one time migration as required to the new code, however it needs to work with older versions of Juju. It also means that we can/look to require new uploads to the charmstore be in the new format. Again, there's some sort of backward/deprecation notice I feel like we have to have some idea for. This one seems even trickier though as it's tied to the client and the charmstore supports multiple clients.

Member

johnsca commented Sep 28, 2015

@rogpeppe Ah, that makes sense. So I'm also +1 on SupportedSeries in the internally-used JSON and metadata.yaml using series.

Owner

marcoceppi commented Sep 28, 2015

No one should be using the series field to date in the charm metadata, so having a deprecation notice is fine but will go unheard AFAIK. I'm happy to validate this assertion by going through every charm (promulgated or otherwise) to make sure series isn't in use currently. My primary concern was to keep the series language in the metadata if at all possible. @rogpeppe thanks for the explanation there

Owner

mitechie commented Sep 28, 2015

@marcoceppi I agree the series won't be in the metadata, so I guess I more meant if a user deploys a local charm without the series, but by specifying it another way (repo/cli flag, whatever) it should be called out as a deprecated deploy command and that the charm should have series noted in the metadata.yaml.

Basically help direct the user to the new path is all.

Owner

marcoceppi commented Sep 28, 2015

Gotchya, makes sense

If the 'series' field becomes an officially supported field, please consider adding it to the docs. I've created an issue for that: juju/docs#1184

I guess it will become an officially supported field since the charm publish command talks about it in an error message: ERROR cannot post archive: series not specified in url or charm metadata

Owner

marcoceppi commented Jun 15, 2016

It is supported in Juju 2.0, we will update the docs. Thanks!

On Wed, Jun 15, 2016, 7:08 AM Merlijn Sebrechts notifications@github.com
wrote:

If the 'series' field becomes an officially supported field, please
consider adding it to the docs. I've created an issue for that:
juju/docs#1184 juju/docs#1184

I guess it will become an officially supported field since the charm
publish command talks about it in an error message: ERROR cannot post
archive: series not specified in url or charm metadata


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#156 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAET1WlLr9cJhgm_jE57-xo3cotAr4xoks5qL_kwgaJpZM4GFDo1
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment